launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/bfjo-shrink into lp:launchpad.

Commit message:
Destroy BuildFarmBuildJob, merge BuildFarmJobOld and BuildFarmJobOldDerived.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/bfjo-shrink/+merge/144227

This branch begins tearing down BuildFarmJobOld. I merged it and BuildFarmBuildJob into BuildFarmJobOldDerived, and renamed BuildFarmJobOldDerived to BuildFarmJobOld, as it's all common functionality now.

IBuildFarmBuildJob and IBuildFarmBranchJob remain as primarily marker interfacse for the +current view fragment used on /builders.
-- 
https://code.launchpad.net/~wgrant/launchpad/bfjo-shrink/+merge/144227
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/bfjo-shrink into lp:launchpad.

=== modified file 'lib/lp/buildmaster/model/buildfarmjob.py'
--- lib/lp/buildmaster/model/buildfarmjob.py	2013-01-22 01:42:59 +0000
+++ lib/lp/buildmaster/model/buildfarmjob.py	2013-01-22 06:55:25 +0000
@@ -7,7 +7,6 @@
     'BuildFarmJobDerived',
     'BuildFarmJobMixin',
     'BuildFarmJobOld',
-    'BuildFarmJobOldDerived',
     ]
 
 import datetime
@@ -59,95 +58,13 @@
 
 
 class BuildFarmJobOld:
-    """See `IBuildFarmJobOld`."""
+    """Some common implementation for IBuildFarmJobOld."""
+
     implements(IBuildFarmJobOld)
+
     processor = None
     virtualized = None
 
-    def score(self):
-        """See `IBuildFarmJobOld`."""
-        raise NotImplementedError
-
-    def getLogFileName(self):
-        """See `IBuildFarmJobOld`."""
-        return 'buildlog.txt'
-
-    def getName(self):
-        """See `IBuildFarmJobOld`."""
-        raise NotImplementedError
-
-    def getTitle(self):
-        """See `IBuildFarmJobOld`."""
-        raise NotImplementedError
-
-    def getByJob(self, job):
-        """See `IBuildFarmJobOld`."""
-        raise NotImplementedError
-
-    def getByJobs(self, job):
-        """See `IBuildFarmJobOld`."""
-        raise NotImplementedError
-
-    def jobStarted(self):
-        """See `IBuildFarmJobOld`."""
-        pass
-
-    def jobReset(self):
-        """See `IBuildFarmJobOld`."""
-        pass
-
-    def jobAborted(self):
-        """See `IBuildFarmJobOld`."""
-        pass
-
-    def jobCancel(self):
-        """See `IBuildFarmJobOld`."""
-        pass
-
-    @staticmethod
-    def addCandidateSelectionCriteria(processor, virtualized):
-        """See `IBuildFarmJobOld`."""
-        raise NotImplementedError
-
-    @staticmethod
-    def postprocessCandidate(job, logger):
-        """See `IBuildFarmJobOld`."""
-        raise NotImplementedError
-
-    def cleanUp(self):
-        """See `IBuildFarmJob`."""
-        pass
-
-    def generateSlaveBuildCookie(self):
-        """See `IBuildFarmJobOld`."""
-        raise NotImplementedError
-
-
-class BuildFarmJobOldDerived:
-    """Setup the delegation and provide some common implementation."""
-    delegates(IBuildFarmJobOld, context='build_farm_job')
-
-    def __init__(self, *args, **kwargs):
-        """Ensure the instance to which we delegate is set on creation."""
-        self._set_build_farm_job()
-        super(BuildFarmJobOldDerived, self).__init__(*args, **kwargs)
-
-    def __storm_loaded__(self):
-        """Set the attribute for our IBuildFarmJob delegation.
-
-        This is needed here as __init__() is not called when a storm object
-        is loaded from the database.
-        """
-        self._set_build_farm_job()
-
-    def _set_build_farm_job(self):
-        """Set the build farm job to which we will delegate.
-
-        Deriving classes must set the build_farm_job attribute for the
-        delegation.
-        """
-        raise NotImplementedError
-
     @staticmethod
     def preloadBuildFarmJobs(jobs):
         """Preload the build farm jobs to which the given jobs will delegate.
@@ -170,6 +87,22 @@
         return store.find(
             cls, cls.job_id.is_in(job_ids))
 
+    def score(self):
+        """See `IBuildFarmJobOld`."""
+        raise NotImplementedError
+
+    def getLogFileName(self):
+        """See `IBuildFarmJobOld`."""
+        return 'buildlog.txt'
+
+    def getName(self):
+        """See `IBuildFarmJobOld`."""
+        raise NotImplementedError
+
+    def getTitle(self):
+        """See `IBuildFarmJob`."""
+        return self.build.title
+
     def generateSlaveBuildCookie(self):
         """See `IBuildFarmJobOld`."""
         buildqueue = getUtility(IBuildQueueSet).getByJob(self.job)
@@ -208,6 +141,23 @@
         """See `IBuildFarmJobOld`."""
         return True
 
+    def jobStarted(self):
+        """See `IBuildFarmJobOld`."""
+        # XXX wgrant: builder should be set here.
+        self.build.updateStatus(BuildStatus.BUILDING)
+
+    def jobReset(self):
+        """See `IBuildFarmJob`."""
+        self.build.updateStatus(BuildStatus.NEEDSBUILD)
+
+    def jobAborted(self):
+        """See `IBuildFarmJob`."""
+        self.build.updateStatus(BuildStatus.NEEDSBUILD)
+
+    def jobCancel(self):
+        """See `IBuildFarmJob`."""
+        self.build.updateStatus(BuildStatus.CANCELLED)
+
 
 class BuildFarmJob(Storm):
     """A base implementation for `IBuildFarmJob` classes."""

=== modified file 'lib/lp/code/model/sourcepackagerecipebuild.py'
--- lib/lp/code/model/sourcepackagerecipebuild.py	2013-01-09 07:58:55 +0000
+++ lib/lp/code/model/sourcepackagerecipebuild.py	2013-01-22 06:55:25 +0000
@@ -36,7 +36,7 @@
     BuildFarmJobType,
     BuildStatus,
     )
-from lp.buildmaster.model.buildfarmjob import BuildFarmJobOldDerived
+from lp.buildmaster.model.buildfarmjob import BuildFarmJobOld
 from lp.buildmaster.model.buildqueue import BuildQueue
 from lp.buildmaster.model.packagebuild import (
     PackageBuild,
@@ -71,7 +71,6 @@
 from lp.soyuz.interfaces.archive import CannotUploadToArchive
 from lp.soyuz.model.archive import Archive
 from lp.soyuz.model.binarypackagebuild import BinaryPackageBuild
-from lp.soyuz.model.buildfarmbuildjob import BuildFarmBuildJob
 from lp.soyuz.model.sourcepackagerelease import SourcePackageRelease
 
 
@@ -392,7 +391,7 @@
         return self.requester
 
 
-class SourcePackageRecipeBuildJob(BuildFarmJobOldDerived, Storm):
+class SourcePackageRecipeBuildJob(BuildFarmJobOld, Storm):
     classProvides(ISourcePackageRecipeBuildJobSource)
     implements(ISourcePackageRecipeBuildJob)
 
@@ -421,12 +420,6 @@
         self.job = job
         super(SourcePackageRecipeBuildJob, self).__init__()
 
-    def _set_build_farm_job(self):
-        """Setup the IBuildFarmJob delegate.
-
-        We override this to provide a delegate specific to package builds."""
-        self.build_farm_job = BuildFarmBuildJob(self.build)
-
     @staticmethod
     def preloadBuildFarmJobs(jobs):
         from lp.code.model.sourcepackagerecipebuild import (

=== removed file 'lib/lp/soyuz/model/buildfarmbuildjob.py'
--- lib/lp/soyuz/model/buildfarmbuildjob.py	2013-01-22 04:41:46 +0000
+++ lib/lp/soyuz/model/buildfarmbuildjob.py	1970-01-01 00:00:00 +0000
@@ -1,51 +0,0 @@
-# Copyright 2010-2011 Canonical Ltd.  This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-__metaclass__ = type
-__all__ = [
-    'BuildFarmBuildJob',
-    ]
-
-
-from zope.interface import implements
-
-from lp.buildmaster.enums import BuildStatus
-from lp.buildmaster.model.buildfarmjob import BuildFarmJobOld
-from lp.soyuz.interfaces.buildfarmbuildjob import IBuildFarmBuildJob
-
-
-class BuildFarmBuildJob(BuildFarmJobOld):
-    """See `IBuildFaramBuildJob`."""
-    implements(IBuildFarmBuildJob)
-
-    def __init__(self, build):
-        """Store the build for this package build farm job.
-
-        XXX 2010-04-12 michael.nelson bug=536700
-        The build param will no longer be necessary once BuildFarmJob is
-        itself a concrete class. This class (PackageBuildFarmJob)
-        will also be renamed PackageBuild and turned into a concrete class.
-        """
-        super(BuildFarmBuildJob, self).__init__()
-        self.build = build
-
-    def getTitle(self):
-        """See `IBuildFarmJob`."""
-        return self.build.title
-
-    def jobStarted(self):
-        """See `IBuildFarmJobOld`."""
-        # XXX wgrant: builder should be set here.
-        self.build.updateStatus(BuildStatus.BUILDING)
-
-    def jobReset(self):
-        """See `IBuildFarmJob`."""
-        self.build.updateStatus(BuildStatus.NEEDSBUILD)
-
-    def jobAborted(self):
-        """See `IBuildFarmJob`."""
-        self.build.updateStatus(BuildStatus.NEEDSBUILD)
-
-    def jobCancel(self):
-        """See `IBuildFarmJob`."""
-        self.build.updateStatus(BuildStatus.CANCELLED)

=== modified file 'lib/lp/soyuz/model/buildpackagejob.py'
--- lib/lp/soyuz/model/buildpackagejob.py	2013-01-22 00:33:13 +0000
+++ lib/lp/soyuz/model/buildpackagejob.py	2013-01-22 06:55:25 +0000
@@ -17,7 +17,7 @@
 
 from lp.buildmaster.enums import BuildStatus
 from lp.buildmaster.interfaces.builder import IBuilderSet
-from lp.buildmaster.model.buildfarmjob import BuildFarmJobOldDerived
+from lp.buildmaster.model.buildfarmjob import BuildFarmJobOld
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.services.database.bulk import load_related
 from lp.services.database.lpstorm import IStore
@@ -36,11 +36,10 @@
     SCORE_BY_URGENCY,
     )
 from lp.soyuz.interfaces.packageset import IPackagesetSet
-from lp.soyuz.model.buildfarmbuildjob import BuildFarmBuildJob
 from lp.soyuz.model.packageset import Packageset
 
 
-class BuildPackageJob(BuildFarmJobOldDerived, Storm):
+class BuildPackageJob(BuildFarmJobOld, Storm):
     """See `IBuildPackageJob`."""
     implements(IBuildPackageJob)
 
@@ -57,12 +56,6 @@
         self.build, self.job = build, job
         super(BuildPackageJob, self).__init__()
 
-    def _set_build_farm_job(self):
-        """Setup the IBuildFarmJob delegate.
-
-        We override this to provide a delegate specific to package builds."""
-        self.build_farm_job = BuildFarmBuildJob(self.build)
-
     @staticmethod
     def preloadBuildFarmJobs(jobs):
         from lp.soyuz.model.binarypackagebuild import BinaryPackageBuild

=== modified file 'lib/lp/translations/model/translationtemplatesbuildjob.py'
--- lib/lp/translations/model/translationtemplatesbuildjob.py	2012-06-29 08:40:05 +0000
+++ lib/lp/translations/model/translationtemplatesbuildjob.py	2013-01-22 06:55:25 +0000
@@ -24,10 +24,7 @@
 from lp.buildmaster.interfaces.buildfarmbranchjob import IBuildFarmBranchJob
 from lp.buildmaster.interfaces.buildfarmjob import IBuildFarmJobSource
 from lp.buildmaster.interfaces.buildqueue import IBuildQueueSet
-from lp.buildmaster.model.buildfarmjob import (
-    BuildFarmJobOld,
-    BuildFarmJobOldDerived,
-    )
+from lp.buildmaster.model.buildfarmjob import BuildFarmJobOld
 from lp.buildmaster.model.buildqueue import BuildQueue
 from lp.code.interfaces.branchjob import IRosettaUploadJobSource
 from lp.code.model.branchjob import (
@@ -53,7 +50,7 @@
 HARDCODED_TRANSLATIONTEMPLATESBUILD_SCORE = 2510
 
 
-class TranslationTemplatesBuildJob(BuildFarmJobOldDerived, BranchJobDerived):
+class TranslationTemplatesBuildJob(BuildFarmJobOld, BranchJobDerived):
     """An `IBuildFarmJob` implementation that generates templates.
 
     Implementation-wise, this is actually a `BranchJob`.
@@ -67,16 +64,6 @@
 
     unsafe_chars = '[^a-zA-Z0-9_+-]'
 
-    def __init__(self, branch_job):
-        super(TranslationTemplatesBuildJob, self).__init__(branch_job)
-
-    def _set_build_farm_job(self):
-        """Setup the IBuildFarmJob delegate.
-
-        We override this to provide a non-database delegate that simply
-        provides required functionality to the queue system."""
-        self.build_farm_job = BuildFarmJobOld()
-
     def score(self):
         """See `IBuildFarmJob`."""
         # Hard-code score for now.  Most PPA jobs start out at 2505;