launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/build-cookie-without-bfj-id into lp:launchpad with lp:~wgrant/launchpad/specificbuildfarmjobsource as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/build-cookie-without-bfj-id/+merge/59884

This branch builds on the ISpecificBuildFarmJobSource utilities introduced in lp:~wgrant/launchpad/specificbuildfarmjobsource, using them to replace BuildFarmJob.id as it appears in build upload paths.

buildd-manager uses these paths to communicate to archiveuploader the build to which an upload is targeted. Currently they encode BuildFarmJob.id, but this won't be possible when BuildFarmJob ceases to be a concrete table or class within a few weeks. So I've changed buildd-manager to use (BuildFarmJobType, $BUILDCLASS.id) instead. archiveuploader uses ISpecificBuildFarmJobSource to translate this into the concrete IBuildFarmJob provider.
-- 
https://code.launchpad.net/~wgrant/launchpad/build-cookie-without-bfj-id/+merge/59884
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/build-cookie-without-bfj-id into lp:launchpad.

=== modified file 'lib/lp/archiveuploader/tests/test_uploadprocessor.py'
--- lib/lp/archiveuploader/tests/test_uploadprocessor.py	2011-03-28 10:56:00 +0000
+++ lib/lp/archiveuploader/tests/test_uploadprocessor.py	2011-05-04 08:50:57 +0000
@@ -1865,7 +1865,7 @@
             CannotGetBuild,
             BuildUploadHandler, self.uploadprocessor, upload_dir, cookie)
         self.assertIn(
-            "Unable to find package build job with id 42. Skipping.", str(e))
+            "Unable to find PACKAGEBUILD with id 42. Skipping.", str(e))
 
     def testBinaryPackageBuild_fail(self):
         # If the upload directory is empty, the upload

=== modified file 'lib/lp/archiveuploader/uploadprocessor.py'
--- lib/lp/archiveuploader/uploadprocessor.py	2011-02-02 05:25:32 +0000
+++ lib/lp/archiveuploader/uploadprocessor.py	2011-05-04 08:50:57 +0000
@@ -73,7 +73,7 @@
 from lp.buildmaster.enums import (
     BuildStatus,
     )
-from lp.buildmaster.interfaces.buildfarmjob import IBuildFarmJobSet
+from lp.buildmaster.interfaces.buildfarmjob import ISpecificBuildFarmJobSource
 from lp.code.interfaces.sourcepackagerecipebuild import (
     ISourcePackageRecipeBuild,
     )
@@ -108,9 +108,9 @@
     :param name: Directory name.
     :return: Tuple with build farm job id.
     """
-    (job_id_str,) = name.split("-")[-1:]
+    (job_type, job_id_str) = name.split("-")[-2:]
     try:
-        return int(job_id_str)
+        return (job_type, int(job_id_str))
     except TypeError:
         raise ValueError
 
@@ -620,19 +620,18 @@
 
     def _getBuild(self):
         try:
-            job_id = parse_build_upload_leaf_name(self.upload)
+            job_type, job_id = parse_build_upload_leaf_name(self.upload)
         except ValueError:
             raise CannotGetBuild(
                 "Unable to extract build id from leaf name %s, skipping." %
                 self.upload)
         try:
-            buildfarm_job = getUtility(IBuildFarmJobSet).getByID(job_id)
+            return getUtility(ISpecificBuildFarmJobSource, job_type).getByID(
+                job_id)
         except NotFoundError:
             raise CannotGetBuild(
-                "Unable to find package build job with id %d. Skipping." %
-                job_id)
-            return
-        return buildfarm_job.getSpecificJob()
+                "Unable to find %s with id %d. Skipping." %
+                (job_type, job_id))
 
     def process(self):
         """Process an upload that is the result of a build.

=== modified file 'lib/lp/buildmaster/model/packagebuild.py'
--- lib/lp/buildmaster/model/packagebuild.py	2011-05-03 03:14:57 +0000
+++ lib/lp/buildmaster/model/packagebuild.py	2011-05-04 08:50:57 +0000
@@ -161,12 +161,6 @@
         timestamp = now.strftime("%Y%m%d-%H%M%S")
         return '%s-%s' % (timestamp, build_cookie)
 
-    def getBuildCookie(self):
-        """See `IPackageBuild`."""
-        return '%s-%s-%s' % (
-            self.id, self.build_farm_job.job_type.name,
-            self.build_farm_job.id)
-
     @staticmethod
     def getLogFromSlave(package_build):
         """See `IPackageBuild`."""
@@ -250,6 +244,10 @@
         """See `IPackageBuild`."""
         raise NotImplementedError
 
+    def getBuildCookie(self):
+        """See `IPackageBuild`."""
+        raise NotImplementedError
+
     def getUploader(self, changes):
         """See `IPackageBuild`."""
         raise NotImplementedError
@@ -263,6 +261,10 @@
     """
     delegates(IPackageBuild, context="package_build")
 
+    def getBuildCookie(self):
+        """See `IPackageBuild`."""
+        return '%s-%s' % (self.job_type.name, self.id)
+
     def queueBuild(self, suspended=False):
         """See `IPackageBuild`."""
         specific_job = self.makeJob()

=== modified file 'lib/lp/buildmaster/tests/test_packagebuild.py'
--- lib/lp/buildmaster/tests/test_packagebuild.py	2011-01-07 00:49:50 +0000
+++ lib/lp/buildmaster/tests/test_packagebuild.py	2011-05-04 08:50:57 +0000
@@ -203,15 +203,6 @@
             '%s-%s' % (now.strftime("%Y%m%d-%H%M%S"), build_cookie),
             upload_leaf)
 
-    def test_getBuildCookie(self):
-        # A build cookie is made up of the package build id and record id.
-        # The uploadprocessor relies on this format.
-        Store.of(self.package_build).flush()
-        cookie = self.package_build.getBuildCookie()
-        expected_cookie = "%d-PACKAGEBUILD-%d" % (
-            self.package_build.id, self.package_build.build_farm_job.id)
-        self.assertEquals(expected_cookie, cookie)
-
     def test_destroySelf_removes_BuildFarmJob(self):
         # Destroying a packagebuild also destroys the BuildFarmJob it
         # references.

=== modified file 'lib/lp/code/model/tests/test_sourcepackagerecipebuild.py'
--- lib/lp/code/model/tests/test_sourcepackagerecipebuild.py	2011-04-13 02:08:31 +0000
+++ lib/lp/code/model/tests/test_sourcepackagerecipebuild.py	2011-05-04 08:50:57 +0000
@@ -120,6 +120,15 @@
             bq.processor)
         self.assertEqual(bq, spb.buildqueue_record)
 
+    def test_getBuildCookie(self):
+        # A build cookie is made up of the job type and record id.
+        # The uploadprocessor relies on this format.
+        sprb = self.makeSourcePackageRecipeBuild()
+        Store.of(sprb).flush()
+        cookie = sprb.getBuildCookie()
+        expected_cookie = "RECIPEBRANCHBUILD-%d" % sprb.id
+        self.assertEquals(expected_cookie, cookie)
+
     def test_title(self):
         # A recipe build's title currently consists of the base
         # branch's unique name.

=== modified file 'lib/lp/soyuz/tests/test_binarypackagebuild.py'
--- lib/lp/soyuz/tests/test_binarypackagebuild.py	2011-05-04 08:50:57 +0000
+++ lib/lp/soyuz/tests/test_binarypackagebuild.py	2011-05-04 08:50:57 +0000
@@ -72,6 +72,14 @@
         self.failIfEqual(None, bq.processor)
         self.failUnless(bq, self.build.buildqueue_record)
 
+    def test_getBuildCookie(self):
+        # A build cookie is made up of the job type and record id.
+        # The uploadprocessor relies on this format.
+        Store.of(self.build).flush()
+        cookie = self.build.getBuildCookie()
+        expected_cookie = "PACKAGEBUILD-%d" % self.build.id
+        self.assertEquals(expected_cookie, cookie)
+
     def test_estimateDuration(self):
         # Without previous builds, a negligable package size estimate is 60s
         self.assertEqual(60, self.build.estimateDuration().seconds)