launchpad-reviewers team mailing list archive

[Данило Шеган <danilo@xxxxxxxxxxxxx>]

Данило Шеган has proposed merging lp:~danilo/launchpad/bug-685624 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #685624 Translation template build jobs should use the new BuildFarmJob
  https://bugs.launchpad.net/bugs/685624


= 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
-- 
https://code.launchpad.net/~danilo/launchpad/bug-685624/+merge/44035
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~danilo/launchpad/bug-685624 into lp:launchpad.

=== modified file 'lib/lp/translations/model/translationtemplatesbuild.py'
--- lib/lp/translations/model/translationtemplatesbuild.py	2010-09-10 11:27:48 +0000
+++ lib/lp/translations/model/translationtemplatesbuild.py	2010-12-17 12:47:57 +0000
@@ -59,7 +59,10 @@
         store = IStore(BranchJob)
 
         # Pass public HTTP URL for the branch.
-        metadata = {'branch_url': self.branch.composePublicURL()}
+        metadata = {
+            'branch_url': self.branch.composePublicURL(),
+            'build_id': self.id,
+            }
         branch_job = BranchJob(
             self.branch, BranchJobType.TRANSLATION_TEMPLATES_BUILD, metadata)
         store.add(branch_job)

=== modified file 'lib/lp/translations/model/translationtemplatesbuildjob.py'
--- lib/lp/translations/model/translationtemplatesbuildjob.py	2010-10-06 18:53:53 +0000
+++ lib/lp/translations/model/translationtemplatesbuildjob.py	2010-12-17 12:47:57 +0000
@@ -105,6 +105,16 @@
         # both try to delete the attached Job.
         Store.of(self.context).remove(self.context)
 
+    @property
+    def build(self):
+        """Return a TranslationTemplateBuild for this build job."""
+        build_id = self.context.metadata.get('build_id', None)
+        if build_id is None:
+            return None
+        else:
+            return getUtility(ITranslationTemplatesBuildSource).get(
+                int(build_id))
+
     @classmethod
     def _hasPotteryCompatibleSetup(cls, branch):
         """Does `branch` look as if pottery can generate templates for it?
@@ -142,7 +152,7 @@
         return True
 
     @classmethod
-    def create(cls, branch):
+    def create(cls, branch, testing=False):
         """See `ITranslationTemplatesBuildJobSource`."""
         logger = logging.getLogger('translation-templates-build')
         # XXX Danilo Segan bug=580429: we hard-code processor to the Ubuntu
@@ -159,6 +169,8 @@
             build_farm_job.id, build.id)
 
         specific_job = build.makeJob()
+        if testing:
+            removeSecurityProxy(specific_job)._constructed_build = build
         logger.debug("Made %s.", specific_job)
 
         duration_estimate = cls.duration_estimate

=== modified file 'lib/lp/translations/tests/test_translationtemplatesbuildjob.py'
--- lib/lp/translations/tests/test_translationtemplatesbuildjob.py	2010-10-04 19:50:45 +0000
+++ lib/lp/translations/tests/test_translationtemplatesbuildjob.py	2010-12-17 12:47:57 +0000
@@ -294,6 +294,12 @@
         tail = branch.name
         self.assertEqual(tail, url[-len(tail):])
 
+    def test_create_with_build(self):
+        branch = self._makeTranslationBranch(fake_pottery_compatible=True)
+        specific_job = self.jobsource.create(branch, testing=True)
+        naked_job = removeSecurityProxy(specific_job)
+        self.assertEquals(naked_job._constructed_build, naked_job.build)
+
 
 def test_suite():
     return TestLoader().loadTestsFromName(__name__)