launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/refactor-ttb-composeBuildRequest into lp:launchpad.

Commit message:
Refactor TTBs to use the common composeBuildRequest implementation.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/refactor-ttb-composeBuildRequest/+merge/361442

This gets us down to a single implementation of IBuildFarmJobBehaviour.composeBuildRequest, which makes it easier to make changes to it.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/refactor-ttb-composeBuildRequest into lp:launchpad.

=== modified file 'lib/lp/buildmaster/interfaces/buildfarmjobbehaviour.py'
--- lib/lp/buildmaster/interfaces/buildfarmjobbehaviour.py	2018-05-02 12:30:22 +0000
+++ lib/lp/buildmaster/interfaces/buildfarmjobbehaviour.py	2019-01-07 14:29:14 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2019 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Interface for build farm job behaviours."""
@@ -21,6 +21,8 @@
         "The name of the builder type to use for this build, corresponding "
         "to a launchpad-buildd build manager tag.")
 
+    archive = Attribute("The `Archive` to build against.")
+
     distro_arch_series = Attribute("The `DistroArchSeries` to build against.")
 
     def setBuilder(builder, slave):

=== modified file 'lib/lp/buildmaster/model/buildfarmjobbehaviour.py'
--- lib/lp/buildmaster/model/buildfarmjobbehaviour.py	2018-05-02 13:22:17 +0000
+++ lib/lp/buildmaster/model/buildfarmjobbehaviour.py	2019-01-07 14:29:14 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2019 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Base and idle BuildFarmJobBehaviour classes."""
@@ -50,6 +50,13 @@
         self._builder = None
 
     @property
+    def archive(self):
+        if self.build is not None:
+            return self.build.archive
+        else:
+            return None
+
+    @property
     def distro_arch_series(self):
         if self.build is not None:
             return self.build.distro_arch_series
@@ -69,7 +76,7 @@
         """The default behaviour is to send only common extra arguments."""
         args = {}
         args["arch_tag"] = self.distro_arch_series.architecturetag
-        args["archive_private"] = self.build.archive.private
+        args["archive_private"] = self.archive.private
         args["build_url"] = canonical_url(self.build)
         args["fast_cleanup"] = self._builder.virtualized
         args["series"] = self.distro_arch_series.distroseries.name

=== modified file 'lib/lp/translations/model/translationtemplatesbuildbehaviour.py'
--- lib/lp/translations/model/translationtemplatesbuildbehaviour.py	2017-11-08 10:57:11 +0000
+++ lib/lp/translations/model/translationtemplatesbuildbehaviour.py	2019-01-07 14:29:14 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010-2017 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2019 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """An `IBuildFarmJobBehaviour` for `TranslationTemplatesBuild`.
@@ -39,6 +39,8 @@
 class TranslationTemplatesBuildBehaviour(BuildFarmJobBehaviourBase):
     """Dispatches `TranslationTemplateBuildJob`s to slaves."""
 
+    builder_type = "translation-templates"
+
     # Filename for the tarball of templates that the slave builds.
     templates_tarball_path = 'translation-templates.tar.gz'
 
@@ -52,19 +54,22 @@
             self.unsafe_chars, '_', self.build.branch.unique_name)
         return "translationtemplates_%s_%d.txt" % (safe_name, self.build.id)
 
-    def composeBuildRequest(self, logger):
-        das = self._getDistroArchSeries()
-        args = {
-            'arch_tag': das.architecturetag,
-            'branch_url': self.build.branch.composePublicURL(),
-            'series': das.distroseries.name,
-            }
-        return ("translation-templates", self._getDistroArchSeries(), {}, args)
+    @property
+    def archive(self):
+        ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
+        return ubuntu.main_archive
 
-    def _getDistroArchSeries(self):
+    @property
+    def distro_arch_series(self):
         ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
         return ubuntu.currentseries.nominatedarchindep
 
+    def extraBuildArgs(self, logger=None):
+        args = super(TranslationTemplatesBuildBehaviour, self).extraBuildArgs(
+            logger=logger)
+        args["branch_url"] = self.build.branch.composePublicURL()
+        return args
+
     def _readTarball(self, buildqueue, filemap, logger):
         """Read tarball with generated translation templates from slave."""
         if filemap is None:

=== modified file 'lib/lp/translations/tests/test_translationtemplatesbuildbehaviour.py'
--- lib/lp/translations/tests/test_translationtemplatesbuildbehaviour.py	2017-12-19 17:22:44 +0000
+++ lib/lp/translations/tests/test_translationtemplatesbuildbehaviour.py	2019-01-07 14:29:14 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010-2017 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2019 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Unit tests for TranslationTemplatesBuildBehaviour."""
@@ -24,6 +24,7 @@
     )
 from lp.services.config import config
 from lp.services.librarian.utils import copy_and_close
+from lp.services.webapp import canonical_url
 from lp.testing import TestCaseWithFactory
 from lp.testing.dbuser import switch_dbuser
 from lp.testing.fakemethod import FakeMethod
@@ -65,7 +66,7 @@
         slave = WaitingSlave(**kwargs)
         behaviour.setBuilder(self.factory.makeBuilder(), slave)
         if use_fake_chroot:
-            behaviour._getDistroArchSeries().addOrUpdateChroot(
+            behaviour.distro_arch_series.addOrUpdateChroot(
                 self.factory.makeLibraryFileAlias(db_only=True))
             self.layer.txn.commit()
         return behaviour
@@ -97,27 +98,38 @@
         b2 = self.makeBehaviour()
         self.assertNotEqual(b1.getLogFileName(), b2.getLogFileName())
 
+    @defer.inlineCallbacks
     def test_composeBuildRequest(self):
         behaviour = self.makeBehaviour()
         switch_dbuser(config.builddmaster.dbuser)
         build_request = yield behaviour.composeBuildRequest(None)
-        das = behaviour._getDistroArchSeries()
+        das = behaviour.distro_arch_series
         self.assertEqual(
             ('translation-templates', das, {}, {
                 'arch_tag': das.architecturetag,
+                'archive_private': False,
                 'branch_url': behaviour.build.branch.composePublicURL(),
+                'build_url': canonical_url(behaviour.build),
+                'fast_cleanup': True,
                 'series': das.distroseries.name,
                 }),
             build_request)
 
-    def test_getDistroArchSeries(self):
-        # _getDistroArchSeries produces the nominated arch-indep
-        # architecture for the current Ubuntu series.
+    def test_archive(self):
+        # TranslationTemplatesBuildBehaviour.archive is the main Ubuntu
+        # archive.
+        ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
+        behaviour = self.makeBehaviour()
+        self.assertEqual(ubuntu.main_archive, behaviour.archive)
+
+    def test_distro_arch_series(self):
+        # TranslationTemplatesBuildBehaviour.distro_arch_series is the
+        # nominated arch-indep architecture for the current Ubuntu series.
         ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
         behaviour = self.makeBehaviour()
         self.assertEqual(
             ubuntu.currentseries.nominatedarchindep,
-            behaviour._getDistroArchSeries())
+            behaviour.distro_arch_series)
 
     def test_readTarball(self):
         behaviour = self.makeBehaviour()