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

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
=== modified file 'lib/lp/buildmaster/interfaces/buildfarmjob.py'
--- lib/lp/buildmaster/interfaces/buildfarmjob.py 2010-04-21 07:15:40 +0000
+++ lib/lp/buildmaster/interfaces/buildfarmjob.py 2010-04-28 10:17:30 +0000
@@ -93,7 +93,7 @@
93class IBuildFarmJobDerived(Interface):93class IBuildFarmJobDerived(Interface):
94 """Common functionality required by classes delegating IBuildFarmJob.94 """Common functionality required by classes delegating IBuildFarmJob.
9595
96 An implementation of this class must setup the necessary delagation.96 An implementation of this class must setup the necessary delegation.
97 """97 """
9898
99 def getByJob(job):99 def getByJob(job):
@@ -149,3 +149,5 @@
149 accurately based on this job's properties.149 accurately based on this job's properties.
150 """150 """
151151
152 def cleanUp():
153 """Job's finished. Delete its supporting data."""
152154
=== modified file 'lib/lp/buildmaster/model/buildfarmjob.py'
--- lib/lp/buildmaster/model/buildfarmjob.py 2010-04-21 07:15:40 +0000
+++ lib/lp/buildmaster/model/buildfarmjob.py 2010-04-28 10:17:30 +0000
@@ -8,14 +8,16 @@
8 ]8 ]
99
1010
11import hashlib
12
11from lazr.delegates import delegates13from lazr.delegates import delegates
1214
13import hashlib
14
15from zope.component import getUtility15from zope.component import getUtility
16from zope.interface import implements16from zope.interface import implements
17from zope.security.proxy import removeSecurityProxy17from zope.security.proxy import removeSecurityProxy
1818
19from storm.store import Store
20
19from canonical.launchpad.webapp.interfaces import (21from canonical.launchpad.webapp.interfaces import (
20 DEFAULT_FLAVOR, IStoreSelector, MAIN_STORE)22 DEFAULT_FLAVOR, IStoreSelector, MAIN_STORE)
2123
@@ -128,4 +130,6 @@
128130
129 return hashlib.sha1(contents).hexdigest()131 return hashlib.sha1(contents).hexdigest()
130132
131133 def cleanUp(self):
134 """See `IBuildFarmJob`."""
135 Store.of(self).remove(self)
132136
=== modified file 'lib/lp/buildmaster/model/buildqueue.py'
--- lib/lp/buildmaster/model/buildqueue.py 2010-03-24 07:17:20 +0000
+++ lib/lp/buildmaster/model/buildqueue.py 2010-04-28 10:17:30 +0000
@@ -18,7 +18,6 @@
1818
19from sqlobject import (19from sqlobject import (
20 StringCol, ForeignKey, BoolCol, IntCol, IntervalCol, SQLObjectNotFound)20 StringCol, ForeignKey, BoolCol, IntCol, IntervalCol, SQLObjectNotFound)
21from storm.store import Store
22from zope.component import getSiteManager, getUtility21from zope.component import getSiteManager, getUtility
23from zope.interface import implements22from zope.interface import implements
2423
@@ -130,7 +129,7 @@
130 job = self.job129 job = self.job
131 specific_job = self.specific_job130 specific_job = self.specific_job
132 SQLBase.destroySelf(self)131 SQLBase.destroySelf(self)
133 Store.of(specific_job).remove(specific_job)132 specific_job.cleanUp()
134 job.destroySelf()133 job.destroySelf()
135134
136 def manualScore(self, value):135 def manualScore(self, value):
137136
=== modified file 'lib/lp/buildmaster/model/packagebuildfarmjob.py'
--- lib/lp/buildmaster/model/packagebuildfarmjob.py 2010-04-20 14:38:20 +0000
+++ lib/lp/buildmaster/model/packagebuildfarmjob.py 2010-04-28 10:17:30 +0000
@@ -56,5 +56,3 @@
56 """56 """
57 def _set_build_farm_job(self):57 def _set_build_farm_job(self):
58 self._build_farm_job = PackageBuildFarmJob(self.build)58 self._build_farm_job = PackageBuildFarmJob(self.build)
59
60
6159
=== modified file 'lib/lp/translations/model/translationtemplatesbuildjob.py'
--- lib/lp/translations/model/translationtemplatesbuildjob.py 2010-04-16 13:31:48 +0000
+++ lib/lp/translations/model/translationtemplatesbuildjob.py 2010-04-28 10:17:30 +0000
@@ -13,6 +13,8 @@
13from zope.interface import classProvides, implements13from zope.interface import classProvides, implements
14from zope.security.proxy import removeSecurityProxy14from zope.security.proxy import removeSecurityProxy
1515
16from storm.store import Store
17
16from canonical.config import config18from canonical.config import config
1719
18from canonical.launchpad.webapp.interfaces import (20from canonical.launchpad.webapp.interfaces import (
@@ -67,6 +69,14 @@
67 """See `IBuildFarmJob`."""69 """See `IBuildFarmJob`."""
68 return '%s translation templates build' % self.branch.bzr_identity70 return '%s translation templates build' % self.branch.bzr_identity
6971
72 def cleanUp(self):
73 """See `IBuildFarmJob`."""
74 # This class is not itself database-backed. But it delegates to
75 # one that is. We can't call its SQLObject destroySelf method
76 # though, because then the BuildQueue and the BranchJob would
77 # both try to delete the attached Job.
78 Store.of(self.context).remove(self.context)
79
70 @classmethod80 @classmethod
71 def _hasPotteryCompatibleSetup(cls, branch):81 def _hasPotteryCompatibleSetup(cls, branch):
72 """Does `branch` look as if pottery can generate templates for it?82 """Does `branch` look as if pottery can generate templates for it?
7383
=== modified file 'lib/lp/translations/tests/test_translationtemplatesbuildjob.py'
--- lib/lp/translations/tests/test_translationtemplatesbuildjob.py 2010-04-16 13:31:48 +0000
+++ lib/lp/translations/tests/test_translationtemplatesbuildjob.py 2010-04-28 10:17:30 +0000
@@ -9,8 +9,9 @@
9from zope.event import notify9from zope.event import notify
10from zope.security.proxy import removeSecurityProxy10from zope.security.proxy import removeSecurityProxy
1111
12from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities12from sqlobject import SQLObjectNotFound
13from canonical.launchpad.interfaces.librarian import ILibraryFileAliasSet13from storm.store import Store
14
14from canonical.launchpad.webapp.testing import verifyObject15from canonical.launchpad.webapp.testing import verifyObject
15from canonical.launchpad.webapp.interfaces import (16from canonical.launchpad.webapp.interfaces import (
16 DEFAULT_FLAVOR, IStoreSelector, MAIN_STORE)17 DEFAULT_FLAVOR, IStoreSelector, MAIN_STORE)
@@ -20,10 +21,9 @@
2021
21from lp.buildmaster.interfaces.buildfarmjob import (22from lp.buildmaster.interfaces.buildfarmjob import (
22 IBuildFarmJob, IBuildFarmJobDerived)23 IBuildFarmJob, IBuildFarmJobDerived)
23from lp.buildmaster.interfaces.buildfarmjobbehavior import (
24 IBuildFarmJobBehavior)
25from lp.buildmaster.interfaces.buildqueue import IBuildQueueSet24from lp.buildmaster.interfaces.buildqueue import IBuildQueueSet
26from lp.buildmaster.model.buildqueue import BuildQueue25from lp.buildmaster.model.buildqueue import BuildQueue
26from lp.code.interfaces.branch import IBranchSet
27from lp.code.interfaces.branchjob import IBranchJob27from lp.code.interfaces.branchjob import IBranchJob
28from lp.code.model.branchjob import BranchJob28from lp.code.model.branchjob import BranchJob
29from lp.code.model.directbranchcommit import DirectBranchCommit29from lp.code.model.directbranchcommit import DirectBranchCommit
@@ -105,6 +105,29 @@
105 # future however the scoring system is to be revisited.105 # future however the scoring system is to be revisited.
106 self.assertEqual(1000, self.specific_job.score())106 self.assertEqual(1000, self.specific_job.score())
107107
108 def test_cleanUp(self):
109 # TranslationTemplatesBuildJob has its own customized cleanup
110 # behaviour, since it's actually a BranchJob.
111 job = removeSecurityProxy(self.specific_job.job)
112 buildqueue = getUtility(IBuildQueueSet).getByJob(job)
113
114 job_id = job.id
115 store = Store.of(job)
116 branch_name = self.branch.unique_name
117
118 buildqueue.destroySelf()
119
120 # BuildQueue is gone.
121 self.assertIs(
122 None, store.find(BuildQueue, BuildQueue.job == job_id).one())
123 # Job is gone.
124 self.assertIs(None, store.find(Job, Job.id == job_id).one())
125 # TranslationTemplatesBuildJob is gone.
126 self.assertIs(None, TranslationTemplatesBuildJob.getByJob(job_id))
127 # Branch is still here.
128 branch_set = getUtility(IBranchSet)
129 self.assertEqual(self.branch, branch_set.getByUniqueName(branch_name))
130
108131
109class FakeTranslationTemplatesJobSource(TranslationTemplatesBuildJob):132class FakeTranslationTemplatesJobSource(TranslationTemplatesBuildJob):
110 """Fake utility class.133 """Fake utility class.
@@ -255,35 +278,5 @@
255 self.assertEqual(tail, url[-len(tail):])278 self.assertEqual(tail, url[-len(tail):])
256279
257280
258class TestTranslationTemplatesBuildBehavior(TestCaseWithFactory):
259 """Test `TranslationTemplatesBuildBehavior`."""
260
261 layer = ZopelessDatabaseLayer
262
263 def setUp(self):
264 super(TestTranslationTemplatesBuildBehavior, self).setUp()
265 self.jobset = getUtility(ITranslationTemplatesBuildJobSource)
266 self.branch = self.factory.makeBranch()
267 self.specific_job = self.jobset.create(self.branch)
268 self.behavior = IBuildFarmJobBehavior(self.specific_job)
269
270 def test_getChroot(self):
271 # _getChroot produces the current chroot for the current Ubuntu
272 # release, on the nominated architecture for
273 # architecture-independent builds.
274 ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
275 current_ubuntu = ubuntu.currentseries
276 distroarchseries = current_ubuntu.nominatedarchindep
277
278 # Set an arbitrary chroot file.
279 fake_chroot_file = getUtility(ILibraryFileAliasSet)[1]
280 distroarchseries.addOrUpdateChroot(fake_chroot_file)
281
282 chroot = self.behavior._getChroot()
283
284 self.assertNotEqual(None, chroot)
285 self.assertEqual(fake_chroot_file, chroot)
286
287
288def test_suite():281def test_suite():
289 return TestLoader().loadTestsFromName(__name__)282 return TestLoader().loadTestsFromName(__name__)