launchpad-reviewers team mailing list archive

[Raphaël Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Badin has proposed merging lp:~rvb/launchpad/relatedjobs-bug-891600 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #891600 in Launchpad itself: "InconsistentBuildFarmJobError: Could not find all the related specific jobs. "
  https://bugs.launchpad.net/launchpad/+bug/891600

For more details, see:
https://code.launchpad.net/~rvb/launchpad/relatedjobs-bug-891600/+merge/82870

This branch makes BuildFarmJobSet.getSpecificJobs handle duplicate jobs correctly.  As wgrant points out, it's not normal to have duplicates here but we thought we should fix this first.

= Tests =

./bin/test -vvc test_buildfarmjob test_getSpecificJobs_duplicated_builds

= Q/A =

https://qastaging.launchpad.net/ubuntu/karmic/%2Bsource/libatasmart/%2Bbuilds should not oops anymore.
-- 
https://code.launchpad.net/~rvb/launchpad/relatedjobs-bug-891600/+merge/82870
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/launchpad/relatedjobs-bug-891600 into lp:launchpad.

=== modified file 'lib/lp/buildmaster/model/buildfarmjob.py'
--- lib/lp/buildmaster/model/buildfarmjob.py	2011-11-11 15:53:21 +0000
+++ lib/lp/buildmaster/model/buildfarmjob.py	2011-11-21 12:31:30 +0000
@@ -70,6 +70,7 @@
     )
 from lp.buildmaster.interfaces.buildqueue import IBuildQueueSet
 from lp.registry.model.teammembership import TeamParticipation
+from lp.soyuz.interfaces.binarypackagebuild import IBinaryPackageBuildSet
 
 
 class BuildFarmJobOld:
@@ -404,23 +405,27 @@
         builds = []
         key = attrgetter('job_type.name')
         sorted_jobs = sorted(jobs, key=key)
+        job_builds = {}
         for job_type_name, grouped_jobs in groupby(sorted_jobs, key=key):
             # Fetch the jobs in batches grouped by their job type.
             source = getUtility(
                 ISpecificBuildFarmJobSource, job_type_name)
-            builds.extend(
-                [build for build
-                    in source.getByBuildFarmJobs(list(grouped_jobs))
-                    if build is not None])
-        # Make sure that all the specific jobs have been found.
-        if len(jobs) != len(builds):
+            builds = [build for build
+                in source.getByBuildFarmJobs(list(grouped_jobs))
+                if build is not None]
+            is_binary_package_build = IBinaryPackageBuildSet.providedBy(
+                source)
+            for build in builds:
+                if is_binary_package_build:
+                    job_builds[build.package_build.build_farm_job.id] = build
+                else:
+                    job_builds[build.build_farm_job.id] = build
+        # Return the corresponding builds.
+        try:
+            return [job_builds[job.id] for job in jobs]
+        except KeyError:
             raise InconsistentBuildFarmJobError(
                 "Could not find all the related specific jobs.")
-        # Sort the builds to match the jobs' order.
-        sorted_builds = sorted(
-            builds,
-            key=lambda build: list(jobs).index(build.build_farm_job))
-        return sorted_builds
 
     def getBuildsForBuilder(self, builder_id, status=None, user=None):
         """See `IBuildFarmJobSet`."""

=== modified file 'lib/lp/buildmaster/tests/test_buildfarmjob.py'
--- lib/lp/buildmaster/tests/test_buildfarmjob.py	2011-11-14 09:04:47 +0000
+++ lib/lp/buildmaster/tests/test_buildfarmjob.py	2011-11-21 12:31:30 +0000
@@ -275,6 +275,13 @@
             [(build.id, build.__class__) for build in builds],
             [(job.id, job.__class__) for job in specific_jobs])
 
+    def test_getSpecificJobs_duplicated_builds(self):
+        builds = self.createBuilds()
+        duplicated_builds = builds + builds
+        specific_jobs = self.build_farm_job_set.getSpecificJobs(
+            [build.build_farm_job for build in duplicated_builds])
+        self.assertEqual(len(duplicated_builds), len(specific_jobs))
+
     def test_getSpecificJobs_empty(self):
         self.assertContentEqual(
             [],