launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/build-score-threshold-arch into lp:launchpad.

Commit message:
Allow setting processor-specific minimum build scores.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/build-score-threshold-arch/+merge/335934
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/build-score-threshold-arch into lp:launchpad.

=== modified file 'lib/lp/buildmaster/model/builder.py'
--- lib/lp/buildmaster/model/builder.py	2018-01-08 17:52:08 +0000
+++ lib/lp/buildmaster/model/builder.py	2018-01-10 12:33:01 +0000
@@ -252,16 +252,28 @@
                         BuildFarmJob.job_type != job_type,
                         Exists(SQL(query))))
 
+        def get_int_feature_flag(flag):
+            value_str = getFeatureFlag(flag)
+            if value_str is not None:
+                try:
+                    return int(value_str)
+                except ValueError:
+                    logger.error('invalid %s %r', flag, value_str)
+
         score_conditions = []
-        minimum_score_str = getFeatureFlag('buildmaster.minimum_score')
-        if minimum_score_str is not None:
-            try:
-                minimum_score = int(minimum_score_str)
-            except ValueError:
-                logger.error(
-                    'invalid buildmaster.minimum_score %r', minimum_score_str)
-            else:
-                score_conditions.append(BuildQueue.lastscore >= minimum_score)
+        minimum_scores = set()
+        for processor in self.processors:
+            minimum_scores.add(get_int_feature_flag(
+                'buildmaster.minimum_score.%s' % processor.name))
+        minimum_scores.add(get_int_feature_flag('buildmaster.minimum_score'))
+        minimum_scores.discard(None)
+        # If there are minimum scores set for any of the processors
+        # supported by this builder, use the highest of them.  This is a bit
+        # weird and not completely ideal, but it's a safe conservative
+        # option and avoids substantially complicating the candidate query.
+        if minimum_scores:
+            score_conditions.append(
+                BuildQueue.lastscore >= max(minimum_scores))
 
         store = IStore(self.__class__)
         candidate_jobs = store.using(BuildQueue, BuildFarmJob).find(

=== modified file 'lib/lp/buildmaster/tests/test_builder.py'
--- lib/lp/buildmaster/tests/test_builder.py	2018-01-08 17:52:08 +0000
+++ lib/lp/buildmaster/tests/test_builder.py	2018-01-10 12:33:01 +0000
@@ -196,10 +196,12 @@
         bq2.manualScore(99999)
         builder1 = removeSecurityProxy(
             self.factory.makeBuilder(
-                processors=[bq1.processor], virtualized=True))
+                processors=[bq1.processor, self.factory.makeProcessor()],
+                virtualized=True))
         builder2 = removeSecurityProxy(
             self.factory.makeBuilder(
-                processors=[bq2.processor], virtualized=True))
+                processors=[bq2.processor, self.factory.makeProcessor()],
+                virtualized=True))
 
         # By default, each builder has the appropriate one of the two builds
         # we just created as a candidate.
@@ -212,6 +214,26 @@
             self.assertEqual(bq1, builder1._findBuildCandidate())
             self.assertIsNone(builder2._findBuildCandidate())
 
+        # We can similarly set a minimum score for individual processors.
+        # The maximum of these for any processor supported by the builder is
+        # used.
+        cases = [
+            ({0: '99999'}, bq2),
+            ({1: '99999'}, bq2),
+            ({0: '100000'}, None),
+            ({1: '100000'}, None),
+            ({0: '99999', 1: '99999'}, bq2),
+            ({0: '99999', 1: '100000'}, None),
+            ({0: '100000', 1: '99999'}, None),
+            ]
+        for feature_spec, expected_bq in cases:
+            features = {
+                'buildmaster.minimum_score.%s' % builder2.processors[i].name:
+                    score
+                for i, score in feature_spec.items()}
+            with FeatureFixture(features):
+                self.assertEqual(expected_bq, builder2._findBuildCandidate())
+
         # If we set an invalid minimum score, buildd-manager doesn't
         # explode.
         with FakeLogger() as logger: