Name component builds by states

This patch attemps to make code shorter and easier to read by naming
component builds for different states.

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

module_build_service/models.py

@@ -29,6 +29,7 @@
 import contextlib
 import hashlib
 import json
+import koji
 import re
 from collections import OrderedDict, namedtuple
 from datetime import datetime
@@ -1351,6 +1352,37 @@ class ComponentBuild(MBSBase):
             self.state_reason,
         )
 
+    @property
+    def is_completed(self):
+        return self.state == koji.BUILD_STATES["COMPLETE"]
+
+    @property
+    def is_building(self):
+        return self.state == koji.BUILD_STATES["BUILDING"]
+
+    @property
+    def is_failed(self):
+        return self.state == koji.BUILD_STATES["FAILED"]
+
+    @property
+    def is_waiting_for_build(self):
+        return self.state is None
+
+    @property
+    def is_unbuilt(self):
+        return self.is_waiting_for_build or self.is_building
+
+    @property
+    def is_tagged(self):
+        return self.tagged and (self.tagged_in_final or self.build_time_only)
+
+    @property
+    def is_unsuccessful(self):
+        return (
+            self.state == koji.BUILD_STATES["FAILED"]
+            or self.state == koji.BUILD_STATES["CANCELED"]
+        )
+
 
 class ComponentBuildTrace(MBSBase):
     __tablename__ = "component_builds_trace"

module_build_service/scheduler/handlers/components.py

@@ -79,22 +79,17 @@ def _finalize(config, db_session, msg, state):
 
     further_work = []
 
+    parent_current_batch = parent.current_batch()
+
     # If there are no other components still building in a batch,
     # we can tag all successfully built components in the batch.
     unbuilt_components_in_batch = [
-        c for c in parent.current_batch()
+        c for c in parent_current_batch
-        if c.state == koji.BUILD_STATES["BUILDING"] or not c.state
+        if c.is_waiting_for_build or c.is_building
     ]
     if not unbuilt_components_in_batch:
-        failed_components_in_batch = [
+        failed_components_in_batch = [c for c in parent_current_batch if c.is_unsuccessful]
-            c for c in parent.current_batch()
+        built_components_in_batch = [c for c in parent_current_batch if c.is_completed]
-            if (c.state in [koji.BUILD_STATES["FAILED"], koji.BUILD_STATES["CANCELED"]])
-        ]
-
-        built_components_in_batch = [
-            c for c in parent.current_batch()
-            if c.state == koji.BUILD_STATES["COMPLETE"]
-        ]
 
         builder = module_build_service.builder.GenericBuilder.create_from_module(
             db_session, parent, config
@@ -151,7 +146,8 @@ def _finalize(config, db_session, msg, state):
                 builder.tag_artifacts(component_nvrs_to_tag_in_dest)
 
         db_session.commit()
-    elif any([c.state != koji.BUILD_STATES["BUILDING"] for c in unbuilt_components_in_batch]):
+
+    elif any(not c.is_building for c in unbuilt_components_in_batch):
         # We are not in the middle of the batch building and
         # we have some unbuilt components in this batch. We might hit the
         # concurrent builds threshold in previous call of continue_batch_build
@@ -162,6 +158,7 @@ def _finalize(config, db_session, msg, state):
             db_session, parent, config)
         further_work += module_build_service.utils.continue_batch_build(
             config, parent, db_session, builder)
+
     return further_work
 
 

module_build_service/scheduler/handlers/modules.py

@@ -78,11 +78,6 @@ def failed(config, db_session, msg):
         # This is ok.. it's a race condition we can ignore.
         pass
 
-    unbuilt_components = [
-        c for c in build.component_builds
-        if (c.state != koji.BUILD_STATES["COMPLETE"] and c.state != koji.BUILD_STATES["FAILED"])
-    ]
-
     if build.koji_tag:
         builder = module_build_service.builder.GenericBuilder.create_from_module(
             db_session, build, config)
@@ -90,7 +85,7 @@ def failed(config, db_session, msg):
         if build.new_repo_task_id:
             builder.cancel_build(build.new_repo_task_id)
 
-        for component in unbuilt_components:
+        for component in (c for c in build.component_builds if c.is_unbuilt):
             if component.task_id:
                 builder.cancel_build(component.task_id)
             component.state = koji.BUILD_STATES["FAILED"]
@@ -442,7 +437,7 @@ def wait(config, db_session, msg):
             component_build.state = state
             component_build.reason = reason
             component_build.nvr = nvr
-    elif component_build.state != koji.BUILD_STATES["COMPLETE"]:
+    elif not component_build.is_completed:
         # It's possible that the build succeeded in the builder but some other step failed which
         # caused module-build-macros to be marked as failed in MBS, so check to see if it exists
         # first

module_build_service/scheduler/handlers/repos.py

@@ -25,7 +25,6 @@
 
 import module_build_service.builder
 import logging
-import koji
 from datetime import datetime
 from module_build_service import models, log
 from module_build_service.utils import start_next_batch_build
@@ -54,41 +53,33 @@ def done(config, db_session, msg):
         log.info("Ignoring repo regen for already failed %r" % module_build)
         return
 
+    # If there are no components in this module build, then current_batch will be empty
+    if module_build.component_builds:
+        current_batch = module_build.current_batch()
+    else:
+        current_batch = []
+
     # Get the list of untagged components in current/previous batches which
     # have been built successfully
-    if config.system in ("koji", "test") and module_build.component_builds:
+    if config.system in ("koji", "test") and current_batch:
-        untagged_components = [
+        if any(c.is_completed and not c.is_tagged for c in module_build.up_to_current_batch()):
-            c for c in module_build.up_to_current_batch()
-            if (not c.tagged or (not c.tagged_in_final and not c.build_time_only))
-            and c.state == koji.BUILD_STATES["COMPLETE"]
-        ]
-        if untagged_components:
             log.info("Ignoring repo regen, because not all components are tagged.")
             return
-        if all([c.state is None for c in module_build.current_batch()]):
+        if all(c.is_waiting_for_build for c in current_batch):
             log.info("Ignoring repo regen because no components have started in the batch.")
             return
 
-    # If there are no components in this module build, then current_batch will be empty
-    if not module_build.component_builds:
-        current_batch = []
-    else:
-        current_batch = module_build.current_batch()
-
     # If any in the current batch are still running.. just wait.
-    running = [c.state == koji.BUILD_STATES["BUILDING"] for c in current_batch]
+    running = [c for c in current_batch if c.is_building]
-    if any(running):
+    if running:
         log.info(
-            "%r has %r of %r components still "
+            "%r has %r of %r components still building in this batch (%r total)",
-            "building in this batch (%r total)"
+            module_build, len(running), len(current_batch), len(module_build.component_builds)
-            % (module_build, len(running), len(current_batch), len(module_build.component_builds))
         )
         return
 
     # Assemble the list of all successful components in the batch.
-    good = [c for c in current_batch if c.state == koji.BUILD_STATES["COMPLETE"]]
+    good = [c for c in current_batch if c.is_completed]
-
-    failed_states = (koji.BUILD_STATES["FAILED"], koji.BUILD_STATES["CANCELED"])
 
     # If *none* of the components completed for this batch, then obviously the
     # module fails.  However!  We shouldn't reach this scenario.  There is
@@ -97,7 +88,7 @@ def done(config, db_session, msg):
     # valve.
     if module_build.component_builds and not good:
         state_reason = "Component(s) {} failed to build.".format(
-            ", ".join(c.package for c in current_batch if c.state in failed_states))
+            ", ".join(c.package for c in current_batch if c.is_unsuccessful))
         module_build.transition(
             db_session, config, models.BUILD_STATES["failed"], state_reason, failure_type="infra")
         db_session.commit()
@@ -127,9 +118,8 @@ def done(config, db_session, msg):
     # So now we can either start a new batch if there are still some to build
     # or, if everything is built successfully, then we can bless the module as
     # complete.
-    has_unbuilt_components = any(
+    has_unbuilt_components = any(c.is_unbuilt for c in module_build.component_builds)
-        c.state in [None, koji.BUILD_STATES["BUILDING"]] for c in module_build.component_builds)
+    has_failed_components = any(c.is_unsuccessful for c in module_build.component_builds)
-    has_failed_components = any(c.state in failed_states for c in module_build.component_builds)
 
     further_work = []
     if has_unbuilt_components and not has_failed_components:
@@ -148,7 +138,7 @@ def done(config, db_session, msg):
         if has_failed_components:
             state_reason = "Component(s) {} failed to build.".format(
                 ", ".join(
-                    c.package for c in module_build.component_builds if c.state in failed_states
+                    c.package for c in module_build.component_builds if c.is_unsuccessful
                 )
             )
             module_build.transition(

module_build_service/scheduler/handlers/tags.py

@@ -58,37 +58,21 @@ def tagged(config, db_session, msg):
         component.tagged_in_final = True
     db_session.commit()
 
-    unbuilt_components_in_batch = [
+    if any(c.is_unbuilt for c in module_build.current_batch()):
-        c for c in module_build.current_batch()
-        if c.state == koji.BUILD_STATES["BUILDING"] or not c.state
-    ]
-    if unbuilt_components_in_batch:
         log.info(
             "Not regenerating repo for tag %s, there are still building components in a batch",
             tag,
         )
         return []
 
-    # Get the list of untagged components in current/previous batches which
-    # have been built successfully.
-    untagged_components = [
-        c for c in module_build.up_to_current_batch()
-        if (not c.tagged or (not c.tagged_in_final and not c.build_time_only))
-        and c.state == koji.BUILD_STATES["COMPLETE"]
-    ]
-
     further_work = []
 
     # If all components are tagged, start newRepo task.
-    if not untagged_components:
+    if not any(c.is_completed and not c.is_tagged for c in module_build.up_to_current_batch()):
         builder = module_build_service.builder.GenericBuilder.create_from_module(
             db_session, module_build, config)
 
-        unbuilt_components = [
+        if any(c.is_unbuilt for c in module_build.component_builds):
-            c for c in module_build.component_builds
-            if c.state == koji.BUILD_STATES["BUILDING"] or not c.state
-        ]
-        if unbuilt_components:
             if not _is_new_repo_generating(module_build, builder.koji_session):
                 repo_tag = builder.module_build_tag["name"]
                 log.info("All components in batch tagged, regenerating repo for tag %s", repo_tag)

module_build_service/scheduler/producer.py

@@ -163,10 +163,7 @@ class MBSProducer(PollingProducer):
             for module in stale_module_builds:
                 log.info("{0!r} is stale and is being cleaned up".format(module))
                 # Find completed artifacts in the stale build
-                artifacts = [
+                artifacts = [c for c in module.component_builds if c.is_completed]
-                    c for c in module.component_builds
-                    if c.state == koji.BUILD_STATES["COMPLETE"]
-                ]
                 # If there are no completed artifacts, then there is nothing to tag
                 if artifacts:
                     # Set buildroot_connect=False so it doesn't recreate the Koji target and etc.

module_build_service/utils/batches.py

@@ -93,7 +93,7 @@ def start_build_component(db_session, builder, c):
             db_session.commit()
         return
 
-    if not c.task_id and c.state == koji.BUILD_STATES["BUILDING"]:
+    if not c.task_id and c.is_building:
         c.state = koji.BUILD_STATES["FAILED"]
         c.state_reason = "Failed to build artifact %s: Builder did not return task ID" % (c.package)
         with BUILD_COMPONENT_DB_SESSION_LOCK:
@@ -116,13 +116,8 @@ def continue_batch_build(config, module, db_session, builder, components=None):
     # if none are provided then we just select everything that hasn't
     # successfully built yet or isn't currently being built.
     unbuilt_components = components or [
-        c for c in module.component_builds
+        c for c in module.current_batch()
-        if (
+        if not c.is_completed and not c.is_building and not c.is_failed
-            c.state != koji.BUILD_STATES["COMPLETE"]
-            and c.state != koji.BUILD_STATES["BUILDING"]
-            and c.state != koji.BUILD_STATES["FAILED"]
-            and c.batch == module.batch
-        )
     ]
 
     if not unbuilt_components:
@@ -141,7 +136,7 @@ def continue_batch_build(config, module, db_session, builder, components=None):
     # Check for builds that exist in the build system but MBS doesn't know about
     for component in unbuilt_components:
         # Only evaluate new components
-        if component.state is not None:
+        if not component.is_waiting_for_build:
             continue
         msgs = builder.recover_orphaned_artifact(component)
         further_work += msgs
@@ -149,7 +144,7 @@ def continue_batch_build(config, module, db_session, builder, components=None):
     for c in unbuilt_components:
         # If a previous build of the component was found, then the state will be marked as
         # COMPLETE so we should skip this
-        if c.state == koji.BUILD_STATES["COMPLETE"]:
+        if c.is_completed:
             continue
         # Check the concurrent build threshold.
         if at_concurrent_component_threshold(config, db_session):
@@ -189,47 +184,23 @@ def start_next_batch_build(config, module, db_session, builder, components=None)
 
     :return: a list of BaseMessage instances to be handled by the MBSConsumer.
     """
-    import koji  # Placed here to avoid py2/py3 conflicts...
 
-    # Check the status of the module build and current batch so we can
+    if not any(c.is_unbuilt for c in module.component_builds):
-    # later decide if we can start new batch or not.
-    has_unbuilt_components = False
-    has_unbuilt_components_in_batch = False
-    has_building_components_in_batch = False
-    has_failed_components = False
-    # This is used to determine if it's worth checking if a component can be reused
-    # later on in the code
-    all_reused_in_prev_batch = True
-    for c in module.component_builds:
-        if c.state in [None, koji.BUILD_STATES["BUILDING"]]:
-            has_unbuilt_components = True
-
-            if c.batch == module.batch:
-                if not c.state:
-                    has_unbuilt_components_in_batch = True
-                elif c.state == koji.BUILD_STATES["BUILDING"]:
-                    has_building_components_in_batch = True
-        elif c.state in [koji.BUILD_STATES["FAILED"], koji.BUILD_STATES["CANCELED"]]:
-            has_failed_components = True
-
-        if c.batch == module.batch and not c.reused_component_id:
-            all_reused_in_prev_batch = False
-
-    # Do not start new batch if there are no components to build.
-    if not has_unbuilt_components:
         log.debug(
             "Not starting new batch, there is no component to build for module %s" % module)
         return []
 
-    # Check that there is something to build in current batch before starting
+    current_batch = module.current_batch()
+
+    # Check that if there is something to build in current batch before starting
     # the new one. If there is, continue building current batch.
-    if has_unbuilt_components_in_batch:
+    if any(c.is_unbuilt for c in current_batch):
         log.info("Continuing building batch %d", module.batch)
         return continue_batch_build(config, module, db_session, builder, components)
 
     # Check that there are no components in BUILDING state in current batch.
     # If there are, wait until they are built.
-    if has_building_components_in_batch:
+    if any(c.is_building for c in current_batch):
         log.debug(
             "Not starting new batch, there are still components in "
             "BUILDING state in current batch for module %s",
@@ -239,7 +210,7 @@ def start_next_batch_build(config, module, db_session, builder, components=None)
 
     # Check that there are no failed components in this batch. If there are,
     # do not start the new batch.
-    if has_failed_components:
+    if any(c.is_unsuccessful for c in module.component_builds):
         log.info("Not starting new batch, there are failed components for module %s", module)
         return []
 
@@ -268,12 +239,17 @@ def start_next_batch_build(config, module, db_session, builder, components=None)
     # Find out if there is repo regeneration in progress for this module.
     # If there is, wait until the repo is regenerated before starting a new
     # batch.
-    artifacts = [c.nvr for c in module.current_batch()]
+    artifacts = [c.nvr for c in current_batch]
     if not builder.buildroot_ready(artifacts):
         log.info(
             "Not starting new batch, not all of %r are in the buildroot. Waiting." % artifacts)
         return []
 
+    # This is used to determine if it's worth checking if a component can be
+    # reused later on in the code
+    all_reused_in_prev_batch = all(
+        c.reused_component_id is not None for c in module.component_builds)
+
     # Although this variable isn't necessary, it is easier to read code later on with it
     prev_batch = module.batch
     module.batch += 1
@@ -282,13 +258,8 @@ def start_next_batch_build(config, module, db_session, builder, components=None)
     # if none are provided then we just select everything that hasn't
     # successfully built yet or isn't currently being built.
     unbuilt_components = components or [
-        c for c in module.component_builds
+        c for c in module.current_batch()
-        if (
+        if not c.is_completed and not c.is_building and not c.is_failed
-            c.state != koji.BUILD_STATES["COMPLETE"]
-            and c.state != koji.BUILD_STATES["BUILDING"]
-            and c.state != koji.BUILD_STATES["FAILED"]
-            and c.batch == module.batch
-        )
     ]
 
     # If there are no components to build, skip the batch and start building

module_build_service/utils/submit.py

@@ -910,7 +910,6 @@ def submit_module_build(db_session, username, mmd, params):
     :rtype: list with ModuleBuild
     :return: List with submitted module builds.
     """
-    import koji  # Placed here to avoid py2/py3 conflicts...
     from .mse import generate_expanded_mmds
 
     log.debug(
@@ -975,7 +974,7 @@ def submit_module_build(db_session, username, mmd, params):
             log.debug("Resuming existing module build %r" % module)
             # Reset all component builds that didn't complete
             for component in module.component_builds:
-                if component.state is not None and component.state != koji.BUILD_STATES["COMPLETE"]:
+                if not component.is_waiting_for_build and not component.is_completed:
                     component.state = None
                     component.state_reason = None
                     db_session.add(component)