launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/setupCompleteBuilds-nonspecific into lp:launchpad.

Commit message:
Fix setupCompleteBuilds/getSpecificJobs to handle already specific jobs without extra queries.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/setupCompleteBuilds-nonspecific/+merge/146751

setupCompleteBuilds was querying BPB.build_farm_job to give to getSpecificJobs to get the BPB back, which is sorta pointless. I've reworked getSpecificJobs to just hand already specific jobs back unchanged, so .build_farm_job remains unaccessed and doesn't generate queries.
-- 
https://code.launchpad.net/~wgrant/launchpad/setupCompleteBuilds-nonspecific/+merge/146751
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/setupCompleteBuilds-nonspecific into lp:launchpad.

=== modified file 'lib/lp/soyuz/browser/build.py'
--- lib/lp/soyuz/browser/build.py	2013-01-07 07:43:32 +0000
+++ lib/lp/soyuz/browser/build.py	2013-02-05 23:52:28 +0000
@@ -31,7 +31,10 @@
     Interface,
     )
 from zope.security.interfaces import Unauthorized
-from zope.security.proxy import removeSecurityProxy
+from zope.security.proxy import (
+    isinstance as zope_isinstance,
+    removeSecurityProxy,
+    )
 
 from lp import _
 from lp.app.browser.launchpadform import (
@@ -48,6 +51,7 @@
     ISpecificBuildFarmJobSource,
     )
 from lp.buildmaster.interfaces.packagebuild import IPackageBuild
+from lp.buildmaster.model.buildfarmjob import BuildFarmJob
 from lp.code.interfaces.sourcepackagerecipebuild import (
     ISourcePackageRecipeBuildSource,
     )
@@ -464,9 +468,7 @@
     Return a list of built CompleteBuild instances, or empty
     list if no builds were contained in the received batch.
     """
-    builds = getSpecificJobs(
-        [build.build_farm_job if IPackageBuild.providedBy(build) else build
-            for build in batch])
+    builds = getSpecificJobs(batch)
     if not builds:
         return []
 
@@ -495,12 +497,15 @@
 def getSpecificJobs(jobs):
     """Return the specific build jobs associated with each of the jobs
         in the provided job list.
+
+    If the job is already a specific job, it will be returned unchanged.
     """
     builds = []
     key = attrgetter('job_type.name')
-    sorted_jobs = sorted(jobs, key=key)
+    nonspecific_jobs = sorted(
+        (job for job in jobs if zope_isinstance(job, BuildFarmJob)), key=key)
     job_builds = {}
-    for job_type_name, grouped_jobs in groupby(sorted_jobs, key=key):
+    for job_type_name, grouped_jobs in groupby(nonspecific_jobs, key=key):
         # Fetch the jobs in batches grouped by their job type.
         source = getUtility(
             ISpecificBuildFarmJobSource, job_type_name)
@@ -520,7 +525,9 @@
                 job_builds[naked_build.build_farm_job.id] = None
     # Return the corresponding builds.
     try:
-        return [job_builds[job.id] for job in jobs]
+        return [
+            job_builds[job.id]
+            if zope_isinstance(job, BuildFarmJob) else job for job in jobs]
     except KeyError:
         raise InconsistentBuildFarmJobError(
             "Could not find all the related specific jobs.")