launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #14889
[Merge] lp:~wgrant/launchpad/divorce-bfj-and-bfjo into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/divorce-bfj-and-bfjo into lp:launchpad.
Commit message:
Revert the partial and unclear merging of IBuildFarmJob and IBuildFarmJob old; the objects serve different purposes.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/divorce-bfj-and-bfjo/+merge/141856
This branch divorces the odd partial marriage between IBuildFarmJob and IBuildFarmJobOld, so we can disentangle and eventually remerge them.
BinaryPackageBuild, the permanent record of a build, is an IBuildFarmJob. BuildPackageJob, the temporary link between BinaryPacakgeBuild and BuildQueue, is an IBuildFarmJobOld. These are fairly different objects, obviously, yet IBuildFarmJob inherited IBuildFarmJobOld for reasons unclear. They were easily split and various unneeded methods removed.
--
https://code.launchpad.net/~wgrant/launchpad/divorce-bfj-and-bfjo/+merge/141856
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/divorce-bfj-and-bfjo into lp:launchpad.
=== modified file 'lib/lp/buildmaster/doc/buildfarmjob.txt'
--- lib/lp/buildmaster/doc/buildfarmjob.txt 2010-08-30 02:07:38 +0000
+++ lib/lp/buildmaster/doc/buildfarmjob.txt 2013-01-04 06:19:23 +0000
@@ -14,23 +14,3 @@
>>> buildfarmjob = BuildFarmJob(BuildFarmJobType.PACKAGEBUILD)
>>> verifyObject(IBuildFarmJob, buildfarmjob)
True
-
-The BuildFarmJob class provides many of the common attributes used by
-different types of build farm jobs, as well as default implementations
-for the common methods. The BuildFarmJob class provides default
-implementations for many build-related methods.
-
- >>> print buildfarmjob.getLogFileName()
- buildlog.txt
-
- >>> buildfarmjob.getName()
- Traceback (most recent call last):
- ...
- NotImplementedError
-
-For more details, please refer to lp.buildmaster.tests.test_buildfarmjob.
-
-This class also includes the getByJob() class method used to find the
-instance of a specific build-farm job implementation associated with a
-given Job instance which must be called in the context of a concrete
-derived class. See lp.buildmaster.tests.test_buildqueue for examples.
=== modified file 'lib/lp/buildmaster/interfaces/buildfarmjob.py'
--- lib/lp/buildmaster/interfaces/buildfarmjob.py 2012-04-06 22:46:26 +0000
+++ lib/lp/buildmaster/interfaces/buildfarmjob.py 2013-01-04 06:19:23 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2013 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
# pylint: disable-msg=E0211,E0213
@@ -60,6 +60,7 @@
IBuildFarmJob) once all the corresponding *Build classes and the
BuildQueue have been transitioned to the new database schema.
"""
+
processor = Reference(
IProcessor, title=_("Processor"), required=False, readonly=True,
description=_(
@@ -157,24 +158,28 @@
accurately based on this job's properties.
"""
- def makeJob():
- """Create the specific job relating this with an lp.services.job.
-
- XXX 2010-04-26 michael.nelson bug=567922
- Once all *Build classes are using BuildFarmJob we can lose the
- 'specific_job' attributes and simply have a reference to the
- services job directly on the BuildFarmJob.
- """
-
def cleanUp():
"""Job's finished. Delete its supporting data."""
-class IBuildFarmJob(IBuildFarmJobOld):
+class IBuildFarmJob(Interface):
"""Operations that jobs for the build farm must implement."""
id = Attribute('The build farm job ID.')
+ processor = Reference(
+ IProcessor, title=_("Processor"), required=False, readonly=True,
+ description=_(
+ "The Processor required by this build farm job. "
+ "This should be None for processor-independent job types."))
+
+ virtualized = Bool(
+ title=_('Virtualized'), required=False, readonly=True,
+ description=_(
+ "The virtualization setting required by this build farm job. "
+ "This should be None for job types that do not care whether "
+ "they run virtualized."))
+
date_created = exported(
Datetime(
title=_("Date created"), required=True, readonly=True,
@@ -266,6 +271,15 @@
returned.
"""
+ def makeJob():
+ """Create the specific job relating this with an lp.services.job.
+
+ XXX 2010-04-26 michael.nelson bug=567922
+ Once all *Build classes are using BuildFarmJob we can lose the
+ 'specific_job' attributes and simply have a reference to the
+ services job directly on the BuildFarmJob.
+ """
+
def gotFailure():
"""Increment the failure_count for this job."""
=== modified file 'lib/lp/buildmaster/model/buildfarmjob.py'
--- lib/lp/buildmaster/model/buildfarmjob.py 2012-09-28 06:25:44 +0000
+++ lib/lp/buildmaster/model/buildfarmjob.py 2013-01-04 06:19:23 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2013 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -51,7 +51,6 @@
ISpecificBuildFarmJobSource,
)
from lp.buildmaster.interfaces.buildqueue import IBuildQueueSet
-from lp.services.database.constants import UTC_NOW
from lp.services.database.enumcol import DBEnum
from lp.services.database.interfaces import (
DEFAULT_FLAVOR,
@@ -67,8 +66,6 @@
class BuildFarmJobOld:
"""See `IBuildFarmJobOld`."""
implements(IBuildFarmJobOld)
- processor = None
- virtualized = None
def score(self):
"""See `IBuildFarmJobOld`."""
@@ -86,10 +83,6 @@
"""See `IBuildFarmJobOld`."""
raise NotImplementedError
- def makeJob(self):
- """See `IBuildFarmJobOld`."""
- raise NotImplementedError
-
def getByJob(self, job):
"""See `IBuildFarmJobOld`."""
raise NotImplementedError
@@ -219,7 +212,7 @@
return True
-class BuildFarmJob(BuildFarmJobOld, Storm):
+class BuildFarmJob(Storm):
"""A base implementation for `IBuildFarmJob` classes."""
__storm_table__ = 'BuildFarmJob'
@@ -299,37 +292,6 @@
"""See `IBuildFarmJobOld`."""
raise NotImplementedError
- def jobStarted(self):
- """See `IBuildFarmJob`."""
- self.status = BuildStatus.BUILDING
- # The build started, set the start time if not set already.
- self.date_started = UTC_NOW
- if self.date_first_dispatched is None:
- self.date_first_dispatched = UTC_NOW
-
- def jobReset(self):
- """See `IBuildFarmJob`."""
- self.status = BuildStatus.NEEDSBUILD
- self.date_started = None
-
- # The implementation of aborting a job is the same as resetting
- # a job.
- jobAborted = jobReset
-
- def jobCancel(self):
- """See `IBuildFarmJob`."""
- self.status = BuildStatus.CANCELLED
-
- @staticmethod
- def addCandidateSelectionCriteria(processor, virtualized):
- """See `IBuildFarmJob`."""
- return ('')
-
- @staticmethod
- def postprocessCandidate(job, logger):
- """See `IBuildFarmJob`."""
- return True
-
@property
def buildqueue_record(self):
"""See `IBuildFarmJob`."""
@@ -353,15 +315,6 @@
"""
return None
- def cleanUp(self):
- """See `IBuildFarmJobOld`.
-
- XXX 2010-05-04 michael.nelson bug=570939
- This can be removed once IBuildFarmJobOld is no longer used
- and services jobs are linked directly to IBuildFarmJob.
- """
- pass
-
@property
def was_built(self):
"""See `IBuild`"""
=== modified file 'lib/lp/buildmaster/tests/test_buildfarmjob.py'
--- lib/lp/buildmaster/tests/test_buildfarmjob.py 2011-12-30 06:14:56 +0000
+++ lib/lp/buildmaster/tests/test_buildfarmjob.py 2013-01-04 06:19:23 +0000
@@ -111,45 +111,8 @@
def test_unimplemented_methods(self):
# A build farm job leaves the implementation of various
# methods for derived classes.
- self.assertRaises(NotImplementedError, self.build_farm_job.score)
- self.assertRaises(NotImplementedError, self.build_farm_job.getName)
- self.assertRaises(NotImplementedError, self.build_farm_job.getTitle)
self.assertRaises(NotImplementedError, self.build_farm_job.makeJob)
- def test_jobStarted(self):
- # Starting a job sets the date_started and status, as well as
- # the date first dispatched, if it is the first dispatch of
- # this job.
- self.build_farm_job.jobStarted()
- self.assertTrue(self.build_farm_job.date_first_dispatched is not None)
- self.assertTrue(self.build_farm_job.date_started is not None)
- self.assertEqual(
- BuildStatus.BUILDING, self.build_farm_job.status)
-
- def test_jobReset(self):
- # Resetting a job sets its status back to NEEDSBUILD and unsets
- # the date_started.
- self.build_farm_job.jobStarted()
- self.build_farm_job.jobReset()
- self.failUnlessEqual(
- BuildStatus.NEEDSBUILD, self.build_farm_job.status)
- self.failUnless(self.build_farm_job.date_started is None)
-
- def test_jobAborted(self):
- # Aborting a job sets its status back to NEEDSBUILD and unsets
- # the date_started.
- self.build_farm_job.jobStarted()
- self.build_farm_job.jobAborted()
- self.failUnlessEqual(
- BuildStatus.NEEDSBUILD, self.build_farm_job.status)
- self.failUnless(self.build_farm_job.date_started is None)
-
- def test_jobCancel(self):
- # Cancelling a job sets its status to CANCELLED.
- self.build_farm_job.jobStarted()
- self.build_farm_job.jobCancel()
- self.assertEqual(BuildStatus.CANCELLED, self.build_farm_job.status)
-
def test_title(self):
# The default title simply uses the job type's title.
self.assertEqual(
@@ -159,10 +122,11 @@
def test_duration_none(self):
# If either start or finished is none, the duration will be
# none.
- self.build_farm_job.jobStarted()
+ removeSecurityProxy(self.build_farm_job).date_started = (
+ datetime.now(pytz.UTC))
self.failUnlessEqual(None, self.build_farm_job.duration)
- self.build_farm_job.jobAborted()
+ removeSecurityProxy(self.build_farm_job).date_started = None
removeSecurityProxy(self.build_farm_job).date_finished = (
datetime.now(pytz.UTC))
self.failUnlessEqual(None, self.build_farm_job.duration)
=== modified file 'lib/lp/buildmaster/tests/test_packagebuild.py'
--- lib/lp/buildmaster/tests/test_packagebuild.py 2012-02-09 23:09:36 +0000
+++ lib/lp/buildmaster/tests/test_packagebuild.py 2013-01-04 06:19:23 +0000
@@ -92,8 +92,8 @@
self.assertEqual(self.package_build, retrieved_build)
def test_unimplemented_methods(self):
- # Classes deriving from PackageBuild must provide getTitle.
- self.assertRaises(NotImplementedError, self.package_build.getTitle)
+ # Classes deriving from PackageBuild must provide various
+ # methods.
self.assertRaises(
NotImplementedError, self.package_build.estimateDuration)
self.assertRaises(
=== modified file 'lib/lp/security.py'
--- lib/lp/security.py 2013-01-02 06:26:34 +0000
+++ lib/lp/security.py 2013-01-04 06:19:23 +0000
@@ -59,7 +59,6 @@
IBuilder,
IBuilderSet,
)
-from lp.buildmaster.interfaces.buildfarmbranchjob import IBuildFarmBranchJob
from lp.buildmaster.interfaces.buildfarmjob import (
IBuildFarmJob,
IBuildFarmJobOld,
@@ -202,7 +201,6 @@
from lp.soyuz.interfaces.binarypackagerelease import (
IBinaryPackageReleaseDownloadCount,
)
-from lp.soyuz.interfaces.buildfarmbuildjob import IBuildFarmBuildJob
from lp.soyuz.interfaces.distroarchseries import IDistroArchSeries
from lp.soyuz.interfaces.packagecopyjob import IPlainPackageCopyJob
from lp.soyuz.interfaces.packageset import (
@@ -236,6 +234,9 @@
ITranslationImportQueueEntry,
)
from lp.translations.interfaces.translationsperson import ITranslationsPerson
+from lp.translations.interfaces.translationtemplatesbuild import (
+ ITranslationTemplatesBuild,
+ )
from lp.translations.interfaces.translator import (
IEditTranslator,
ITranslator,
@@ -1982,38 +1983,29 @@
return auth_spr.checkUnauthenticated()
+class ViewTranslationTemplatesBuild(DelegatedAuthorization):
+ """Permission to view an `ITranslationTemplatesBuild`.
+
+ Delegated to the build's branch.
+ """
+ permission = 'launchpad.View'
+ usedfor = ITranslationTemplatesBuild
+
+ def __init__(self, obj):
+ super(ViewTranslationTemplatesBuild, self).__init__(obj, obj.branch)
+
+
class ViewBuildFarmJobOld(DelegatedAuthorization):
"""Permission to view an `IBuildFarmJobOld`.
This permission is based entirely on permission to view the
- associated `IBinaryPackageBuild` and/or `IBranch`.
+ associated `IBuildFarmJob`.
"""
permission = 'launchpad.View'
usedfor = IBuildFarmJobOld
- def _getBranch(self):
- """Get `IBranch` associated with this job, if any."""
- if IBuildFarmBranchJob.providedBy(self.obj):
- return self.obj.branch
- else:
- return None
-
- def _getBuild(self):
- """Get `IPackageBuild` associated with this job, if any."""
- if IBuildFarmBuildJob.providedBy(self.obj):
- return self.obj.build
- else:
- return None
-
- def iter_objects(self):
- branch = self._getBranch()
- build = self._getBuild()
- objects = []
- if branch:
- objects.append(branch)
- if build:
- objects.append(build)
- return objects
+ def __init__(self, obj):
+ super(ViewBuildFarmJobOld, self).__init__(obj, obj.build)
class AdminQuestion(AdminByAdminsTeam):
