From 5f795407667f8106338d4f8d6ebaeef782218d84 Mon Sep 17 00:00:00 2001
From: Chenxiong Qi <cqi@redhat.com>
Date: Dec 08 2017 09:04:52 +0000
Subject: Refactor method to rebuild images


Code to rebuild a list of selected builds is repeated twice. And they
are slightly different to handle the task ID returned from
build_container.

This patch moves those duplicate code to a single method and ensure to
rebuild images in consisntent behavior.

Signed-off-by: Chenxiong Qi <cqi@redhat.com>

---

diff --git a/freshmaker/handlers/__init__.py b/freshmaker/handlers/__init__.py
index 942a5ed..941d4d1 100644
--- a/freshmaker/handlers/__init__.py
+++ b/freshmaker/handlers/__init__.py
@@ -25,6 +25,7 @@ import abc
 import json
 import re
 import itertools
+import six
 from functools import wraps
 
 from freshmaker import conf, log, db, models
@@ -394,19 +395,17 @@ class ContainerBuildHandler(BaseHandler):
 
         return repo_urls
 
-    def _build_first_batch(self, db_event):
-        """
-        Rebuilds all the parents images - images in the first batch which don't
-        depend on other images.
-        """
+    def start_to_build_images(self, builds):
+        """Start to build images
 
-        builds = db.session.query(ArtifactBuild).filter_by(
-            type=ArtifactType.IMAGE.value, event_id=db_event.id,
-            dep_on=None).all()
+        :param builds: list of ArtifactBuild, each of them represents a
+            container image to be rebuilt.
+        :type builds: list or tuple
+        """
 
-        for build in builds:
+        def build_image(build):
             self.set_context(build)
-            repo_urls = self.get_repo_urls(db_event, build)
+            repo_urls = self.get_repo_urls(build.event, build)
             build.build_id = self.build_image_artifact_build(build, repo_urls)
             if build.build_id:
                 build.transition(
@@ -419,4 +418,16 @@ class ContainerBuildHandler(BaseHandler):
             db.session.add(build)
             db.session.commit()
 
+        list(six.moves.map(build_image, builds))
+
+    def _build_first_batch(self, db_event):
+        """
+        Rebuilds all the parents images - images in the first batch which don't
+        depend on other images.
+        """
+
+        builds = db.session.query(ArtifactBuild).filter_by(
+            type=ArtifactType.IMAGE.value, event_id=db_event.id,
+            dep_on=None).all()
+        self.start_to_build_images(builds)
         self.set_context(db_event)
diff --git a/freshmaker/handlers/brew/container_task_state_change.py b/freshmaker/handlers/brew/container_task_state_change.py
index 7b5c0d2..00b1e66 100644
--- a/freshmaker/handlers/brew/container_task_state_change.py
+++ b/freshmaker/handlers/brew/container_task_state_change.py
@@ -39,15 +39,18 @@ class BrewContainerTaskStateChangeHandler(ContainerBuildHandler):
     @fail_event_on_handler_exception
     def handle(self, event):
         """
-        When build container task state changed in brew, update build state in db and
-        rebuild containers depend on the success build as necessary.
+        When build container task state changed in brew, update build state in
+        db and rebuild containers depend on the success build as necessary.
         """
 
         build_id = event.task_id
 
         # check db to see whether this build exists in db
-        found_build = db.session.query(ArtifactBuild).filter_by(type=ArtifactType.IMAGE.value,
-                                                                build_id=build_id).first()
+        found_build = db.session.query(ArtifactBuild).filter_by(
+            type=ArtifactType.IMAGE.value,
+            build_id=build_id
+        ).first()
+
         if found_build is not None:
             self.set_context(found_build)
             # update build state in db
@@ -62,17 +65,20 @@ class BrewContainerTaskStateChangeHandler(ContainerBuildHandler):
             db.session.commit()
 
             if found_build.state == ArtifactBuildState.DONE.value:
-                # check db to see whether there is any planned image build depends on this build
-                planned_builds = db.session.query(ArtifactBuild).filter_by(type=ArtifactType.IMAGE.value,
-                                                                           state=ArtifactBuildState.PLANNED.value,
-                                                                           dep_on=found_build).all()
+                # check db to see whether there is any planned image build
+                # depends on this build
+                planned_builds = db.session.query(ArtifactBuild).filter_by(
+                    type=ArtifactType.IMAGE.value,
+                    state=ArtifactBuildState.PLANNED.value,
+                    dep_on=found_build
+                ).all()
+
+                log.info("Found following PLANNED builds to rebuild that "
+                         "depends on %r", found_build)
                 for build in planned_builds:
-                    self.set_context(build)
-                    repo_urls = self.get_repo_urls(found_build.event, build)
-                    log.info("Build %r depends on build %r" % (build, found_build))
-                    build.build_id = self.build_image_artifact_build(build, repo_urls)
-                    build.state = ArtifactBuildState.BUILD.value
-                db.session.commit()
+                    log.info("  %r", build)
+
+                self.start_to_build_images(planned_builds)
 
             # Finally, we check if all builds scheduled by event
             # found_build.event (ErrataAdvisoryRPMsSignedEvent) have been