Merge lp:~danilo/launchpad/bug-685624 into lp:launchpad

Proposed by Данило Шеган on 2010-12-17
Status: Merged
Approved by: Данило Шеган on 2010-12-17
Approved revision: no longer in the source branch.
Merged at revision: 12098
Proposed branch: lp:~danilo/launchpad/bug-685624
Merge into: lp:launchpad
Diff against target: 71 lines (+23/-2)
3 files modified
lib/lp/translations/model/translationtemplatesbuild.py (+4/-1)
lib/lp/translations/model/translationtemplatesbuildjob.py (+13/-1)
lib/lp/translations/tests/test_translationtemplatesbuildjob.py (+6/-0)
To merge this branch: bzr merge lp:~danilo/launchpad/bug-685624
Reviewer Review Type Date Requested Status
Brad Crittenden (community) code 2010-12-17 Approve on 2010-12-17
Review via email: mp+44035@code.launchpad.net

Commit Message

[r=bac][ui=none][bug=685624] Provide TranslationTemplatesBuildJob.build property linking to TranslationTemplatesBuild objects to stop polluting build-manager logs.

Description of the Change

= Bug 685624 =

Translation template generation happens on the build farm and is triggered by a commit to a branch for upstream projects.

TranslationTemplatesBuildJob (TTBJ) is at the same time a BranchJob (runs on commits) and a BuildFarmJobOld (old-style build farm job, used for queueing template generation jobs on the build farm). Since we are in the middle of the migration to the new model for the build farm, translations also provides a new-style BuildFarmJob in TranslationTemplatesBuild (TTB). However, there is no way to get the new-style BFJ from the old-style one, yet buildmaster currently expects that. This won't be needed in the future when the full migration is done.

== Proposed fix ==

There is no algorithmic way to get from TTBJ to the TTB, so we have to store the link on TTBJ. But TTBJ is both a BranchJob and a BuildFarmJob, and we can't easily extend it to store TTB ID (because it's using the BranchJob table, and we don't want to add the 'build' column there). However, BranchJob provides a generic metadata field in json format as json_data, and we're already using it to pass public branch URL around.

We'll also add TTB.id there as 'build_id', and implement a property TTBJ.build which fetches a TTB based on this 'build_id'.

Note that this is a temporary solution that will not be needed as soon as buildmaster is switched to the new model exclusively.

== Pre-implementation notes ==

I've discussed this with Michael Nelson and William Grant.

== Tests ==

bin/test -cvvt translationtemplatesbuildjob.*create_with_build

== Demo and Q/A ==

I'll still have to check with the Soyuz guys on how do I best QA this. In general, it's about creating a translation templates build job, and then seeing if buildd-manager reports the problem like mentioned in the bug.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/translations/model/translationtemplatesbuildjob.py
  lib/lp/translations/model/translationtemplatesbuild.py
  lib/lp/translations/tests/test_translationtemplatesbuildjob.py

To post a comment you must log in.
Brad Crittenden (bac) wrote :

This branch looks good Danilos. It has a kludgy elegance to it but should suffice until the next pass happens real soon now.

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/translations/model/translationtemplatesbuild.py'
2--- lib/lp/translations/model/translationtemplatesbuild.py 2010-09-10 11:27:48 +0000
3+++ lib/lp/translations/model/translationtemplatesbuild.py 2010-12-17 12:47:57 +0000
4@@ -59,7 +59,10 @@
5 store = IStore(BranchJob)
6
7 # Pass public HTTP URL for the branch.
8- metadata = {'branch_url': self.branch.composePublicURL()}
9+ metadata = {
10+ 'branch_url': self.branch.composePublicURL(),
11+ 'build_id': self.id,
12+ }
13 branch_job = BranchJob(
14 self.branch, BranchJobType.TRANSLATION_TEMPLATES_BUILD, metadata)
15 store.add(branch_job)
16
17=== modified file 'lib/lp/translations/model/translationtemplatesbuildjob.py'
18--- lib/lp/translations/model/translationtemplatesbuildjob.py 2010-10-06 18:53:53 +0000
19+++ lib/lp/translations/model/translationtemplatesbuildjob.py 2010-12-17 12:47:57 +0000
20@@ -105,6 +105,16 @@
21 # both try to delete the attached Job.
22 Store.of(self.context).remove(self.context)
23
24+ @property
25+ def build(self):
26+ """Return a TranslationTemplateBuild for this build job."""
27+ build_id = self.context.metadata.get('build_id', None)
28+ if build_id is None:
29+ return None
30+ else:
31+ return getUtility(ITranslationTemplatesBuildSource).get(
32+ int(build_id))
33+
34 @classmethod
35 def _hasPotteryCompatibleSetup(cls, branch):
36 """Does `branch` look as if pottery can generate templates for it?
37@@ -142,7 +152,7 @@
38 return True
39
40 @classmethod
41- def create(cls, branch):
42+ def create(cls, branch, testing=False):
43 """See `ITranslationTemplatesBuildJobSource`."""
44 logger = logging.getLogger('translation-templates-build')
45 # XXX Danilo Segan bug=580429: we hard-code processor to the Ubuntu
46@@ -159,6 +169,8 @@
47 build_farm_job.id, build.id)
48
49 specific_job = build.makeJob()
50+ if testing:
51+ removeSecurityProxy(specific_job)._constructed_build = build
52 logger.debug("Made %s.", specific_job)
53
54 duration_estimate = cls.duration_estimate
55
56=== modified file 'lib/lp/translations/tests/test_translationtemplatesbuildjob.py'
57--- lib/lp/translations/tests/test_translationtemplatesbuildjob.py 2010-10-04 19:50:45 +0000
58+++ lib/lp/translations/tests/test_translationtemplatesbuildjob.py 2010-12-17 12:47:57 +0000
59@@ -294,6 +294,12 @@
60 tail = branch.name
61 self.assertEqual(tail, url[-len(tail):])
62
63+ def test_create_with_build(self):
64+ branch = self._makeTranslationBranch(fake_pottery_compatible=True)
65+ specific_job = self.jobsource.create(branch, testing=True)
66+ naked_job = removeSecurityProxy(specific_job)
67+ self.assertEquals(naked_job._constructed_build, naked_job.build)
68+
69
70 def test_suite():
71 return TestLoader().loadTestsFromName(__name__)