Code review comment for lp://staging/~michael.nelson/launchpad/567922-binarypackagebuild-new-table-1

Revision history for this message
Michael Nelson (michael.nelson) wrote :

On Tue, May 4, 2010 at 2:14 PM, Michael Nelson
<email address hidden> wrote:
...
> I've attached the incremental of the above, but if it's ok with you,
> I'll also do a second incremental to get rid of IBuildFarmJobDerived
> for the same reason you've outlined above.
>

See attached for the removal of the IBuildFarmJobDerived interface as
above. I also realised while doing this that I don't need the separate
mixin, and have instead provided the shared implementation on the
BuildFarmJobDerived/PackageJobDerived classes directly.

tested with:
bin/test -vv -t test_buildfarmjob -t test_packagebuild -t
test_sourcepackagerecipebuild -t test_translationtemplatesbuildjob

-Michael

1=== modified file 'lib/lp/buildmaster/doc/buildfarmjob.txt'
2--- lib/lp/buildmaster/doc/buildfarmjob.txt 2010-04-28 10:59:12 +0000
3+++ lib/lp/buildmaster/doc/buildfarmjob.txt 2010-05-04 13:42:25 +0000
4@@ -29,17 +29,6 @@
5
6 For more details, please refer to lp.buildmaster.tests.test_buildfarmjob.
7
8-BuildFarmJobDerived
9-===================
10-
11-BuildFarmJobDerived exists to encapsulate the functionality required
12-by classes delegating IBuildFarmJob.
13-
14- >>> from lp.buildmaster.interfaces.buildfarmjob import (
15- ... IBuildFarmJobDerived)
16- >>> verifyObject(IBuildFarmJobDerived, BuildFarmJobDerived())
17- True
18-
19 This class also includes the getByJob() class method used to find the
20 instance of a specific build-farm job implementation associated with a
21 given Job instance which must be called in the context of a concrete
22
23=== modified file 'lib/lp/buildmaster/interfaces/buildfarmjob.py'
24--- lib/lp/buildmaster/interfaces/buildfarmjob.py 2010-04-30 16:11:03 +0000
25+++ lib/lp/buildmaster/interfaces/buildfarmjob.py 2010-05-04 13:42:25 +0000
26@@ -11,7 +11,6 @@
27 'IBuildFarmJob',
28 'IBuildFarmJobOld',
29 'IBuildFarmJobSource',
30- 'IBuildFarmJobDerived',
31 'BuildFarmJobType',
32 ]
33
34@@ -106,6 +105,71 @@
35 def jobAborted():
36 """'Job aborted' life cycle event, handle as appropriate."""
37
38+ def addCandidateSelectionCriteria(processor, virtualized):
39+ """Provide a sub-query to refine the candidate job selection.
40+
41+ Return a sub-query to narrow down the list of candidate jobs.
42+ The sub-query will become part of an "outer query" and is free to
43+ refer to the `BuildQueue` and `Job` tables already utilized in the
44+ latter.
45+
46+ Example (please see the `BuildPackageJob` implementation for a
47+ complete example):
48+
49+ SELECT TRUE
50+ FROM Archive, Build, BuildPackageJob, DistroArchSeries
51+ WHERE
52+ BuildPackageJob.job = Job.id AND
53+ ..
54+
55+ :param processor: the type of processor that the candidate jobs are
56+ expected to run on.
57+ :param virtualized: whether the candidate jobs are expected to run on
58+ the `processor` natively or inside a virtual machine.
59+ :return: a string containing a sub-query that narrows down the list of
60+ candidate jobs.
61+ """
62+
63+ def postprocessCandidate(job, logger):
64+ """True if the candidate job is fine and should be dispatched
65+ to a builder, False otherwise.
66+
67+ :param job: The `BuildQueue` instance to be scrutinized.
68+ :param logger: The logger to use.
69+
70+ :return: True if the candidate job should be dispatched
71+ to a builder, False otherwise.
72+ """
73+
74+ def getByJob(job):
75+ """Get the specific `IBuildFarmJob` for the given `Job`.
76+
77+ Invoked on the specific `IBuildFarmJob`-implementing class that
78+ has an entry associated with `job`.
79+ """
80+
81+ def generateSlaveBuildCookie():
82+ """Produce a cookie for the slave as a token of the job it's doing.
83+
84+ The cookie need not be unique, but should be hard for a
85+ compromised slave to guess.
86+
87+ :return: a hard-to-guess ASCII string that can be reproduced
88+ accurately based on this job's properties.
89+ """
90+
91+ def makeJob():
92+ """Create the specific job relating this with an lp.services.job.
93+
94+ XXX 2010-04-26 michael.nelson bug=567922
95+ Once all *Build classes are using BuildFarmJob we can lose the
96+ 'specific_job' attributes and simply have a reference to the
97+ services job directly on the BuildFarmJob.
98+ """
99+
100+ def cleanUp():
101+ """Job's finished. Delete its supporting data."""
102+
103
104 class IBuildFarmJob(IBuildFarmJobOld):
105 """Operations that jobs for the build farm must implement."""
106@@ -179,78 +243,6 @@
107
108 title = exported(TextLine(title=_("Title"), required=False))
109
110- def makeJob():
111- """Create the specific job relating this with an lp.services.job.
112-
113- XXX 2010-04-26 michael.nelson bug=567922
114- Once all *Build classes are using BuildFarmJob we can lose the
115- 'specific_job' attributes and simply have a reference to the
116- services job directly on the BuildFarmJob.
117- """
118-
119-
120-class IBuildFarmJobDerived(Interface):
121- """Common functionality required by classes delegating IBuildFarmJob.
122-
123- An implementation of this class must setup the necessary delegation.
124- """
125-
126- def getByJob(job):
127- """Get the specific `IBuildFarmJob` for the given `Job`.
128-
129- Invoked on the specific `IBuildFarmJob`-implementing class that
130- has an entry associated with `job`.
131- """
132-
133- def addCandidateSelectionCriteria(processor, virtualized):
134- """Provide a sub-query to refine the candidate job selection.
135-
136- Return a sub-query to narrow down the list of candidate jobs.
137- The sub-query will become part of an "outer query" and is free to
138- refer to the `BuildQueue` and `Job` tables already utilized in the
139- latter.
140-
141- Example (please see the `BuildPackageJob` implementation for a
142- complete example):
143-
144- SELECT TRUE
145- FROM Archive, Build, BuildPackageJob, DistroArchSeries
146- WHERE
147- BuildPackageJob.job = Job.id AND
148- ..
149-
150- :param processor: the type of processor that the candidate jobs are
151- expected to run on.
152- :param virtualized: whether the candidate jobs are expected to run on
153- the `processor` natively or inside a virtual machine.
154- :return: a string containing a sub-query that narrows down the list of
155- candidate jobs.
156- """
157-
158- def postprocessCandidate(job, logger):
159- """True if the candidate job is fine and should be dispatched
160- to a builder, False otherwise.
161-
162- :param job: The `BuildQueue` instance to be scrutinized.
163- :param logger: The logger to use.
164-
165- :return: True if the candidate job should be dispatched
166- to a builder, False otherwise.
167- """
168-
169- def generateSlaveBuildCookie():
170- """Produce a cookie for the slave as a token of the job it's doing.
171-
172- The cookie need not be unique, but should be hard for a
173- compromised slave to guess.
174-
175- :return: a hard-to-guess ASCII string that can be reproduced
176- accurately based on this job's properties.
177- """
178-
179- def cleanUp():
180- """Job's finished. Delete its supporting data."""
181-
182
183 class IBuildFarmJobSource(Interface):
184 """A utility of BuildFarmJob used to create _things_."""
185
186=== modified file 'lib/lp/buildmaster/model/buildfarmjob.py'
187--- lib/lp/buildmaster/model/buildfarmjob.py 2010-04-30 16:12:05 +0000
188+++ lib/lp/buildmaster/model/buildfarmjob.py 2010-05-04 13:42:25 +0000
189@@ -14,7 +14,6 @@
190
191 from lazr.delegates import delegates
192
193-import hashlib
194 import pytz
195
196 from storm.locals import Bool, DateTime, Int, Reference, Storm
197@@ -32,8 +31,8 @@
198
199 from lp.buildmaster.interfaces.buildbase import BuildStatus
200 from lp.buildmaster.interfaces.buildfarmjob import (
201- BuildFarmJobType, IBuildFarmJob, IBuildFarmJobDerived,
202- IBuildFarmJobOld, IBuildFarmJobSource)
203+ BuildFarmJobType, IBuildFarmJob, IBuildFarmJobOld,
204+ IBuildFarmJobSource)
205 from lp.buildmaster.interfaces.buildqueue import IBuildQueueSet
206
207
208@@ -63,6 +62,10 @@
209 """See `IBuildFarmJobOld`."""
210 raise NotImplementedError
211
212+ def getByJob(self):
213+ """See `IBuildFarmJobOld`."""
214+ raise NotImplementedError
215+
216 def jobStarted(self):
217 """See `IBuildFarmJobOld`."""
218 pass
219@@ -75,8 +78,27 @@
220 """See `IBuildFarmJobOld`."""
221 pass
222
223+ @staticmethod
224+ def addCandidateSelectionCriteria(processor, virtualized):
225+ """See `IBuildFarmJobOld`."""
226+ return ('')
227+
228+ @staticmethod
229+ def postprocessCandidate(job, logger):
230+ """See `IBuildFarmJobOld`."""
231+ return True
232+
233+ def cleanUp(self):
234+ """See `IBuildFarmJob`."""
235+ Store.of(self).remove(self)
236+
237+ def generateSlaveBuildCookie(self):
238+ """See `IBuildFarmJobOld`."""
239+ raise NotImplementedError
240+
241
242 class BuildFarmJobOldDerived:
243+ """Setup the delegation and provide some common implementation."""
244 delegates(IBuildFarmJobOld, context='build_farm_job')
245
246 def __init__(self, *args, **kwargs):
247@@ -102,22 +124,12 @@
248
249 @classmethod
250 def getByJob(cls, job):
251- """See `IBuildFarmJobDerived`."""
252+ """See `IBuildFarmJobOld`."""
253 store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
254 return store.find(cls, cls.job == job).one()
255
256- @staticmethod
257- def addCandidateSelectionCriteria(processor, virtualized):
258- """See `IBuildFarmJobDerived`."""
259- return ('')
260-
261- @staticmethod
262- def postprocessCandidate(job, logger):
263- """See `IBuildFarmJobDerived`."""
264- return True
265-
266 def generateSlaveBuildCookie(self):
267- """See `IBuildFarmJobDerived`."""
268+ """See `IBuildFarmJobOld`."""
269 buildqueue = getUtility(IBuildQueueSet).getByJob(self.job)
270
271 if buildqueue.processor is None:
272@@ -136,10 +148,6 @@
273
274 return hashlib.sha1(contents).hexdigest()
275
276- def cleanUp(self):
277- """See `IBuildFarmJob`."""
278- Store.of(self).remove(self)
279-
280
281 class BuildFarmJob(BuildFarmJobOld, Storm):
282 """A base implementation for `IBuildFarmJob` classes."""
283@@ -258,8 +266,16 @@
284 """
285 return None
286
287+ def cleanUp(self):
288+ """See `IBuildFarmJobOld`.
289+
290+ XXX 2010-05-04 michael.nelson bug=570939
291+ This can be removed once IBuildFarmJobOld is no longer used
292+ and services jobs are linked directly to IBuildFarmJob.
293+ """
294+ pass
295+
296
297 class BuildFarmJobDerived:
298- """See `IBuildFarmJobDerived`."""
299- implements(IBuildFarmJobDerived)
300+ implements(IBuildFarmJob)
301 delegates(IBuildFarmJob, context='build_farm_job')
302
303=== modified file 'lib/lp/buildmaster/model/packagebuild.py'
304--- lib/lp/buildmaster/model/packagebuild.py 2010-05-04 12:07:35 +0000
305+++ lib/lp/buildmaster/model/packagebuild.py 2010-05-04 13:47:11 +0000
306@@ -160,17 +160,13 @@
307
308
309 class PackageBuildDerived:
310+ """Setup the delegation for package build.
311+
312+ This class also provides some common implementation for handling
313+ build status.
314+ """
315 delegates(IPackageBuild, context="package_build")
316
317-
318-class PackageBuildStatusHandlerMixin:
319- """A mixin to provide the common implementation of handleStatus.
320-
321- This can be used by classes based on PackageBuild to provide
322- the implementation of handleStatus, ensuring that the methods
323- called during handleStatus are called on the top-level object
324- first.
325- """
326 def handleStatus(self, status, librarian, slave_status):
327 """See `IPackageBuild`."""
328 return BuildBase.handleStatus(self, status, librarian, slave_status)
329
330=== modified file 'lib/lp/code/configure.zcml'
331--- lib/lp/code/configure.zcml 2010-04-30 15:05:53 +0000
332+++ lib/lp/code/configure.zcml 2010-05-04 13:42:25 +0000
333@@ -1024,7 +1024,6 @@
334 <class
335 class="lp.code.model.sourcepackagerecipebuild.SourcePackageRecipeBuildJob">
336 <allow interface="lp.code.interfaces.sourcepackagerecipebuild.ISourcePackageRecipeBuildJob"/>
337- <allow interface="lp.buildmaster.interfaces.buildfarmjob.IBuildFarmJobDerived"/>
338 </class>
339
340 <securedutility
341
342=== modified file 'lib/lp/soyuz/configure.zcml'
343--- lib/lp/soyuz/configure.zcml 2010-04-20 14:30:00 +0000
344+++ lib/lp/soyuz/configure.zcml 2010-05-04 13:42:25 +0000
345@@ -848,8 +848,6 @@
346 class="lp.soyuz.model.buildpackagejob.BuildPackageJob">
347 <allow
348 interface="lp.soyuz.interfaces.buildpackagejob.IBuildPackageJob"/>
349- <allow
350- interface="lp.buildmaster.interfaces.buildfarmjob.IBuildFarmJobDerived"/>
351 </class>
352 <!--
353 The registration below is used to discover all build farm job classes
354
355=== modified file 'lib/lp/soyuz/tests/test_buildpackagejob.py'
356--- lib/lp/soyuz/tests/test_buildpackagejob.py 2010-04-28 13:57:13 +0000
357+++ lib/lp/soyuz/tests/test_buildpackagejob.py 2010-05-04 13:42:25 +0000
358@@ -13,7 +13,6 @@
359
360 from lp.buildmaster.interfaces.buildbase import BuildStatus
361 from lp.buildmaster.interfaces.builder import IBuilderSet
362-from lp.buildmaster.interfaces.buildfarmjob import IBuildFarmJobDerived
363 from lp.soyuz.interfaces.archive import ArchivePurpose
364 from lp.soyuz.interfaces.buildfarmbuildjob import IBuildFarmBuildJob
365 from lp.soyuz.interfaces.buildpackagejob import IBuildPackageJob
366@@ -235,6 +234,3 @@
367 build_farm_job = bq.specific_job
368 self.assertProvides(build_farm_job, IBuildPackageJob)
369 self.assertProvides(build_farm_job, IBuildFarmBuildJob)
370- self.assertProvides(build_farm_job, IBuildFarmJobDerived)
371-
372-
373
374=== modified file 'lib/lp/translations/configure.zcml'
375--- lib/lp/translations/configure.zcml 2010-04-22 07:33:12 +0000
376+++ lib/lp/translations/configure.zcml 2010-05-04 13:42:25 +0000
377@@ -578,9 +578,7 @@
378 <allow
379 interface="lp.code.interfaces.branchjob.IBranchJob"/>
380 <allow
381- interface="lp.buildmaster.interfaces.buildfarmjob.IBuildFarmJob"/>
382- <allow
383- interface="lp.buildmaster.interfaces.buildfarmjob.IBuildFarmJobDerived"/>
384+ interface="lp.buildmaster.interfaces.buildfarmjob.IBuildFarmJobOld"/>
385 </class>
386 <securedutility
387 component="lp.translations.model.translationtemplatesbuildjob.TranslationTemplatesBuildJob"
388
389=== modified file 'lib/lp/translations/model/translationtemplatesbuildjob.py'
390--- lib/lp/translations/model/translationtemplatesbuildjob.py 2010-04-30 16:11:03 +0000
391+++ lib/lp/translations/model/translationtemplatesbuildjob.py 2010-05-04 13:42:25 +0000
392@@ -147,7 +147,7 @@
393
394 @classmethod
395 def getByJob(cls, job):
396- """See `IBuildFarmJobDerived`.
397+ """See `IBuildFarmJob`.
398
399 Overridden here to search via a BranchJob, rather than a Job.
400 """
401
402=== modified file 'lib/lp/translations/tests/test_translationtemplatesbuildjob.py'
403--- lib/lp/translations/tests/test_translationtemplatesbuildjob.py 2010-04-30 16:11:03 +0000
404+++ lib/lp/translations/tests/test_translationtemplatesbuildjob.py 2010-05-04 13:42:25 +0000
405@@ -19,7 +19,7 @@
406 from lp.testing import TestCaseWithFactory
407
408 from lp.buildmaster.interfaces.buildfarmjob import (
409- IBuildFarmJobOld, IBuildFarmJobDerived)
410+ IBuildFarmJobOld)
411 from lp.buildmaster.interfaces.buildqueue import IBuildQueueSet
412 from lp.buildmaster.model.buildqueue import BuildQueue
413 from lp.code.interfaces.branch import IBranchSet
414@@ -54,9 +54,8 @@
415
416 def test_new_TranslationTemplatesBuildJob(self):
417 # TranslationTemplateBuildJob implements IBuildFarmJobOld,
418- # IBuildFarmJobDerived, and IBranchJob.
419+ # and IBranchJob.
420 verifyObject(IBranchJob, self.specific_job)
421- verifyObject(IBuildFarmJobDerived, self.specific_job)
422 verifyObject(IBuildFarmJobOld, self.specific_job)
423
424 # Each of these jobs knows the branch it will operate on.

« Back to merge proposal