Launchpad itself

Merge lp:~jtv/launchpad/bug-569108 into lp:launchpad

  1. bug-569108
  2. Merge into devel
Proposed by Jeroen T. Vermeulen
Status: Merged
Approved by: Jeroen T. Vermeulen
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~jtv/launchpad/bug-569108
Merge into: lp:launchpad
Diff against target: 202 lines (+48/-42)
6 files modified
lib/lp/buildmaster/interfaces/buildfarmjob.py (+3/-1)
lib/lp/buildmaster/model/buildfarmjob.py (+7/-3)
lib/lp/buildmaster/model/buildqueue.py (+1/-2)
lib/lp/buildmaster/model/packagebuildfarmjob.py (+0/-2)
lib/lp/translations/model/translationtemplatesbuildjob.py (+10/-0)
lib/lp/translations/tests/test_translationtemplatesbuildjob.py (+27/-34)
To merge this branch: bzr merge lp:~jtv/launchpad/bug-569108
Reviewer Review Type Date Requested Status
Michael Nelson (community) code Approve
Review via email: mp+24297@code.launchpad.net

Commit message

Fix deletion of TranslationTemplateBuildJobs.

Description of the change

= Bug 569108 =

Work in progress. Fixing up deletion of BuildQueue/IBuildFarmJobDerived/Job when done.

To post a comment you must log in.
Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

Ready for review now. Also passed Q/A on dogfood. No lint. To test:
{{{
./bin/test -vv -t test_translationtemplatesbuildjob
}}}

As a drive-by, I also removed a TranslationTemplatesBuildBehavior test that had already been moved out into a file of its own, but never deleted from the file it was in.

Jeroen

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

Nice clean fix, thanks Jeroen.

I'm not sure how I'll cope with this when I merge it into the refactoring work, where BuildFarmJobs (and their derivations) can be db-backed themselves, but I'll figure that out :)

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/buildmaster/interfaces/buildfarmjob.py'
2--- lib/lp/buildmaster/interfaces/buildfarmjob.py 2010-04-21 07:15:40 +0000
3+++ lib/lp/buildmaster/interfaces/buildfarmjob.py 2010-04-28 10:17:30 +0000
4@@ -93,7 +93,7 @@
5 class IBuildFarmJobDerived(Interface):
6 """Common functionality required by classes delegating IBuildFarmJob.
7
8- An implementation of this class must setup the necessary delagation.
9+ An implementation of this class must setup the necessary delegation.
10 """
11
12 def getByJob(job):
13@@ -149,3 +149,5 @@
14 accurately based on this job's properties.
15 """
16
17+ def cleanUp():
18+ """Job's finished. Delete its supporting data."""
19
20=== modified file 'lib/lp/buildmaster/model/buildfarmjob.py'
21--- lib/lp/buildmaster/model/buildfarmjob.py 2010-04-21 07:15:40 +0000
22+++ lib/lp/buildmaster/model/buildfarmjob.py 2010-04-28 10:17:30 +0000
23@@ -8,14 +8,16 @@
24 ]
25
26
27+import hashlib
28+
29 from lazr.delegates import delegates
30
31-import hashlib
32-
33 from zope.component import getUtility
34 from zope.interface import implements
35 from zope.security.proxy import removeSecurityProxy
36
37+from storm.store import Store
38+
39 from canonical.launchpad.webapp.interfaces import (
40 DEFAULT_FLAVOR, IStoreSelector, MAIN_STORE)
41
42@@ -128,4 +130,6 @@
43
44 return hashlib.sha1(contents).hexdigest()
45
46-
47+ def cleanUp(self):
48+ """See `IBuildFarmJob`."""
49+ Store.of(self).remove(self)
50
51=== modified file 'lib/lp/buildmaster/model/buildqueue.py'
52--- lib/lp/buildmaster/model/buildqueue.py 2010-03-24 07:17:20 +0000
53+++ lib/lp/buildmaster/model/buildqueue.py 2010-04-28 10:17:30 +0000
54@@ -18,7 +18,6 @@
55
56 from sqlobject import (
57 StringCol, ForeignKey, BoolCol, IntCol, IntervalCol, SQLObjectNotFound)
58-from storm.store import Store
59 from zope.component import getSiteManager, getUtility
60 from zope.interface import implements
61
62@@ -130,7 +129,7 @@
63 job = self.job
64 specific_job = self.specific_job
65 SQLBase.destroySelf(self)
66- Store.of(specific_job).remove(specific_job)
67+ specific_job.cleanUp()
68 job.destroySelf()
69
70 def manualScore(self, value):
71
72=== modified file 'lib/lp/buildmaster/model/packagebuildfarmjob.py'
73--- lib/lp/buildmaster/model/packagebuildfarmjob.py 2010-04-20 14:38:20 +0000
74+++ lib/lp/buildmaster/model/packagebuildfarmjob.py 2010-04-28 10:17:30 +0000
75@@ -56,5 +56,3 @@
76 """
77 def _set_build_farm_job(self):
78 self._build_farm_job = PackageBuildFarmJob(self.build)
79-
80-
81
82=== modified file 'lib/lp/translations/model/translationtemplatesbuildjob.py'
83--- lib/lp/translations/model/translationtemplatesbuildjob.py 2010-04-16 13:31:48 +0000
84+++ lib/lp/translations/model/translationtemplatesbuildjob.py 2010-04-28 10:17:30 +0000
85@@ -13,6 +13,8 @@
86 from zope.interface import classProvides, implements
87 from zope.security.proxy import removeSecurityProxy
88
89+from storm.store import Store
90+
91 from canonical.config import config
92
93 from canonical.launchpad.webapp.interfaces import (
94@@ -67,6 +69,14 @@
95 """See `IBuildFarmJob`."""
96 return '%s translation templates build' % self.branch.bzr_identity
97
98+ def cleanUp(self):
99+ """See `IBuildFarmJob`."""
100+ # This class is not itself database-backed. But it delegates to
101+ # one that is. We can't call its SQLObject destroySelf method
102+ # though, because then the BuildQueue and the BranchJob would
103+ # both try to delete the attached Job.
104+ Store.of(self.context).remove(self.context)
105+
106 @classmethod
107 def _hasPotteryCompatibleSetup(cls, branch):
108 """Does `branch` look as if pottery can generate templates for it?
109
110=== modified file 'lib/lp/translations/tests/test_translationtemplatesbuildjob.py'
111--- lib/lp/translations/tests/test_translationtemplatesbuildjob.py 2010-04-16 13:31:48 +0000
112+++ lib/lp/translations/tests/test_translationtemplatesbuildjob.py 2010-04-28 10:17:30 +0000
113@@ -9,8 +9,9 @@
114 from zope.event import notify
115 from zope.security.proxy import removeSecurityProxy
116
117-from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
118-from canonical.launchpad.interfaces.librarian import ILibraryFileAliasSet
119+from sqlobject import SQLObjectNotFound
120+from storm.store import Store
121+
122 from canonical.launchpad.webapp.testing import verifyObject
123 from canonical.launchpad.webapp.interfaces import (
124 DEFAULT_FLAVOR, IStoreSelector, MAIN_STORE)
125@@ -20,10 +21,9 @@
126
127 from lp.buildmaster.interfaces.buildfarmjob import (
128 IBuildFarmJob, IBuildFarmJobDerived)
129-from lp.buildmaster.interfaces.buildfarmjobbehavior import (
130- IBuildFarmJobBehavior)
131 from lp.buildmaster.interfaces.buildqueue import IBuildQueueSet
132 from lp.buildmaster.model.buildqueue import BuildQueue
133+from lp.code.interfaces.branch import IBranchSet
134 from lp.code.interfaces.branchjob import IBranchJob
135 from lp.code.model.branchjob import BranchJob
136 from lp.code.model.directbranchcommit import DirectBranchCommit
137@@ -105,6 +105,29 @@
138 # future however the scoring system is to be revisited.
139 self.assertEqual(1000, self.specific_job.score())
140
141+ def test_cleanUp(self):
142+ # TranslationTemplatesBuildJob has its own customized cleanup
143+ # behaviour, since it's actually a BranchJob.
144+ job = removeSecurityProxy(self.specific_job.job)
145+ buildqueue = getUtility(IBuildQueueSet).getByJob(job)
146+
147+ job_id = job.id
148+ store = Store.of(job)
149+ branch_name = self.branch.unique_name
150+
151+ buildqueue.destroySelf()
152+
153+ # BuildQueue is gone.
154+ self.assertIs(
155+ None, store.find(BuildQueue, BuildQueue.job == job_id).one())
156+ # Job is gone.
157+ self.assertIs(None, store.find(Job, Job.id == job_id).one())
158+ # TranslationTemplatesBuildJob is gone.
159+ self.assertIs(None, TranslationTemplatesBuildJob.getByJob(job_id))
160+ # Branch is still here.
161+ branch_set = getUtility(IBranchSet)
162+ self.assertEqual(self.branch, branch_set.getByUniqueName(branch_name))
163+
164
165 class FakeTranslationTemplatesJobSource(TranslationTemplatesBuildJob):
166 """Fake utility class.
167@@ -255,35 +278,5 @@
168 self.assertEqual(tail, url[-len(tail):])
169
170
171-class TestTranslationTemplatesBuildBehavior(TestCaseWithFactory):
172- """Test `TranslationTemplatesBuildBehavior`."""
173-
174- layer = ZopelessDatabaseLayer
175-
176- def setUp(self):
177- super(TestTranslationTemplatesBuildBehavior, self).setUp()
178- self.jobset = getUtility(ITranslationTemplatesBuildJobSource)
179- self.branch = self.factory.makeBranch()
180- self.specific_job = self.jobset.create(self.branch)
181- self.behavior = IBuildFarmJobBehavior(self.specific_job)
182-
183- def test_getChroot(self):
184- # _getChroot produces the current chroot for the current Ubuntu
185- # release, on the nominated architecture for
186- # architecture-independent builds.
187- ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
188- current_ubuntu = ubuntu.currentseries
189- distroarchseries = current_ubuntu.nominatedarchindep
190-
191- # Set an arbitrary chroot file.
192- fake_chroot_file = getUtility(ILibraryFileAliasSet)[1]
193- distroarchseries.addOrUpdateChroot(fake_chroot_file)
194-
195- chroot = self.behavior._getChroot()
196-
197- self.assertNotEqual(None, chroot)
198- self.assertEqual(fake_chroot_file, chroot)
199-
200-
201 def test_suite():
202 return TestLoader().loadTestsFromName(__name__)