Launchpad itself

Merge lp:~wgrant/launchpad/bug-549340-fix-buildqueue-destruction into lp:launchpad

  1. bug-549340-fix-buildqueue-destruction
  2. Merge into devel
Proposed by William Grant
Status: Rejected
Rejected by: Michael Nelson
Proposed branch: lp:~wgrant/launchpad/bug-549340-fix-buildqueue-destruction
Merge into: lp:launchpad
Prerequisite: lp:~wgrant/launchpad/refactor-slavestatus
Diff against target: 152 lines (+52/-2)
7 files modified
database/schema/security.cfg (+1/-1)
lib/lp/buildmaster/model/buildqueue.py (+1/-1)
lib/lp/code/model/sourcepackagerecipebuild.py (+4/-0)
lib/lp/code/tests/test_sourcepackagerecipebuild.py (+18/-0)
lib/lp/soyuz/interfaces/buildfarmbuildjob.py (+6/-0)
lib/lp/soyuz/model/buildpackagejob.py (+5/-0)
lib/lp/translations/tests/test_translationtemplatesbuildjob.py (+17/-0)
To merge this branch: bzr merge lp:~wgrant/launchpad/bug-549340-fix-buildqueue-destruction
Reviewer Review Type Date Requested Status
Julian Edwards (community) Disapprove
Review via email: mp+22286@code.launchpad.net

Commit message

Fix destruction of translation template build farm jobs.

Description of the change

This branch fixes BuildQueue.destroySelf to not assume that IBuildFarmJobs are Storm objects. TranslationTemplatesBuildJob isn't -- its DB object is actually a BranchJob.

I've replaced the direct store removal with a call to IBFJ.destroySelf, and implemented it on the two classes that needed it. It actually lives on IBuildFarmBuildJob for now, since TTBJ has security declarations for both IBuildFarmJob and IBranchJob, the latter already defining destroySelf -- this results in a Zope permission definition conflict, even though the permissions are actually the same.

I've also added tests for destruction of TTBJ and SPRBJ BuildQueues. That required the granting of DELETE on SPRBJ to launchpad_main.

To post a comment you must log in.
Revision history for this message
Julian Edwards (julian-edwards) wrote :

I do not like this fix, we should not be adding more SQLObject methods. Instead, there should be a real TranslationTemplatesBuildJob table so that build history is preserved.

I'm a bit flummoxed as to how its DB object is a BranchJob?

review: Disapprove

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'database/schema/security.cfg'
2--- database/schema/security.cfg 2010-03-15 01:26:43 +0000
3+++ database/schema/security.cfg 2010-03-27 11:11:27 +0000
4@@ -280,7 +280,7 @@
5 public.sourcepackageformatselection = SELECT
6 public.sourcepackagerecipe = SELECT, INSERT, UPDATE, DELETE
7 public.sourcepackagerecipebuild = SELECT, INSERT, UPDATE, DELETE
8-public.sourcepackagerecipebuildjob = SELECT, INSERT, UPDATE
9+public.sourcepackagerecipebuildjob = SELECT, INSERT, UPDATE, DELETE
10 public.sourcepackagerecipedata = SELECT, INSERT, UPDATE, DELETE
11 public.sourcepackagerecipedatainstruction = SELECT, INSERT, UPDATE, DELETE
12 public.specificationbranch = SELECT, INSERT, UPDATE, DELETE
13
14=== modified file 'lib/lp/buildmaster/model/buildqueue.py'
15--- lib/lp/buildmaster/model/buildqueue.py 2010-03-17 05:58:22 +0000
16+++ lib/lp/buildmaster/model/buildqueue.py 2010-03-27 11:11:27 +0000
17@@ -133,7 +133,7 @@
18 job = self.job
19 specific_job = self.specific_job
20 SQLBase.destroySelf(self)
21- Store.of(specific_job).remove(specific_job)
22+ specific_job.destroySelf()
23 job.destroySelf()
24
25 def manualScore(self, value):
26
27=== modified file 'lib/lp/code/model/sourcepackagerecipebuild.py'
28--- lib/lp/code/model/sourcepackagerecipebuild.py 2010-03-25 15:24:39 +0000
29+++ lib/lp/code/model/sourcepackagerecipebuild.py 2010-03-27 11:11:27 +0000
30@@ -216,3 +216,7 @@
31 store = IMasterStore(SourcePackageRecipeBuildJob)
32 store.add(specific_job)
33 return specific_job
34+
35+ def destroySelf(self):
36+ """See `IBuildFarmJob`."""
37+ Store.of(self).remove(self)
38
39=== modified file 'lib/lp/code/tests/test_sourcepackagerecipebuild.py'
40--- lib/lp/code/tests/test_sourcepackagerecipebuild.py 2010-03-25 15:29:31 +0000
41+++ lib/lp/code/tests/test_sourcepackagerecipebuild.py 2010-03-27 11:11:27 +0000
42@@ -11,6 +11,7 @@
43 import transaction
44 from zope.component import getUtility
45
46+from canonical.launchpad.interfaces.lpstorm import IStore
47 from canonical.testing.layers import DatabaseFunctionalLayer
48
49 from lp.buildmaster.interfaces.buildbase import IBuildBase
50@@ -18,6 +19,8 @@
51 from lp.code.interfaces.sourcepackagerecipebuild import (
52 ISourcePackageRecipeBuildJob, ISourcePackageRecipeBuild,
53 ISourcePackageRecipeBuildSource)
54+from lp.code.model.sourcepackagerecipebuild import (
55+ SourcePackageRecipeBuildJob)
56 from lp.testing import TestCaseWithFactory
57
58
59@@ -92,6 +95,21 @@
60 self.assertEqual(
61 datetime.timedelta(minutes=2), spb.estimateDuration())
62
63+ def test_BuildQueue_destruction(self):
64+ # When destroying the related BuildQueue, the SPRBJ is also
65+ # destroyed.
66+ spb = self.makeSourcePackageRecipeBuild()
67+ bq = spb.queueBuild()
68+ self.assertEqual(
69+ 1,
70+ IStore(SourcePackageRecipeBuildJob).find(
71+ SourcePackageRecipeBuildJob, build=spb).count())
72+ bq.destroySelf()
73+ self.assertEqual(
74+ 0,
75+ IStore(SourcePackageRecipeBuildJob).find(
76+ SourcePackageRecipeBuildJob, build=spb).count())
77+
78
79 def test_suite():
80 return unittest.TestLoader().loadTestsFromName(__name__)
81
82=== modified file 'lib/lp/soyuz/interfaces/buildfarmbuildjob.py'
83--- lib/lp/soyuz/interfaces/buildfarmbuildjob.py 2010-03-16 05:08:47 +0000
84+++ lib/lp/soyuz/interfaces/buildfarmbuildjob.py 2010-03-27 11:11:27 +0000
85@@ -19,3 +19,9 @@
86 build = Reference(
87 IBuild, title=_("Build"), required=True, readonly=True,
88 description=_("Build record associated with this job."))
89+
90+ # XXX wgrant 2010-03-27: This should be on IBuildFarmJob, but
91+ # IBranchJob has this as well so TTBJ security dies with a conflict.
92+ def destroySelf():
93+ """Destroy this job."""
94+
95
96=== modified file 'lib/lp/soyuz/model/buildpackagejob.py'
97--- lib/lp/soyuz/model/buildpackagejob.py 2010-03-25 14:38:25 +0000
98+++ lib/lp/soyuz/model/buildpackagejob.py 2010-03-27 11:11:27 +0000
99@@ -11,6 +11,7 @@
100 import pytz
101
102 from storm.locals import Int, Reference, Storm
103+from storm.store import Store
104
105 from zope.interface import implements
106 from zope.component import getUtility
107@@ -164,6 +165,10 @@
108 """See `IBuildFarmJob`."""
109 return self.build.is_virtualized
110
111+ def destroySelf(self):
112+ """See `IBuildFarmJob`."""
113+ Store.of(self).remove(self)
114+
115 @staticmethod
116 def addCandidateSelectionCriteria(processor, virtualized):
117 """See `IBuildFarmCandidateJobSelection`."""
118
119=== modified file 'lib/lp/translations/tests/test_translationtemplatesbuildjob.py'
120--- lib/lp/translations/tests/test_translationtemplatesbuildjob.py 2010-03-06 00:21:57 +0000
121+++ lib/lp/translations/tests/test_translationtemplatesbuildjob.py 2010-03-27 11:11:27 +0000
122@@ -11,6 +11,7 @@
123
124 from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
125 from canonical.launchpad.interfaces.librarian import ILibraryFileAliasSet
126+from canonical.launchpad.interfaces.lpstorm import IStore
127 from canonical.launchpad.webapp.testing import verifyObject
128 from canonical.launchpad.webapp.interfaces import (
129 DEFAULT_FLAVOR, IStoreSelector, MAIN_STORE)
130@@ -107,6 +108,22 @@
131 # future however the scoring system is to be revisited.
132 self.assertEqual(1000, self.specific_job.score())
133
134+ def test_BuildQueue_destruction(self):
135+ # A TTBJ's BuildQueue can destroy itself, which also destroys
136+ # the related BranchJob.
137+ self.assertEqual(
138+ 1,
139+ IStore(BranchJob).find(
140+ BranchJob, BranchJob.branch == self.branch).count())
141+
142+ buildqueue = getUtility(IBuildQueueSet).getByJob(
143+ self.specific_job.job)
144+ buildqueue.destroySelf()
145+
146+ self.assertEqual(
147+ 0,
148+ IStore(BranchJob).find(BranchJob, branch=self.branch).count())
149+
150
151 class FakeTranslationTemplatesJobSource(TranslationTemplatesBuildJob):
152 """Fake utility class.