launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/ttbj-comply into lp:launchpad.

Commit message:
Modernise TranslationTemplatesBuild like BPB/SPRB. Use updateStatus, kill storeBuildInfo, revoke attribute write access.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/ttbj-comply/+merge/144250

Apply the BPB/SPRB treatment to TTB. storeBuildInfo is dead, status is set through BFJ.updateStatus, updateBuild_WAITING uses inlineCallbacks, and attribute write permissions are revoked.
-- 
https://code.launchpad.net/~wgrant/launchpad/ttbj-comply/+merge/144250
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/ttbj-comply into lp:launchpad.

=== modified file 'lib/lp/buildmaster/model/buildfarmjobbehavior.py'
--- lib/lp/buildmaster/model/buildfarmjobbehavior.py	2013-01-22 02:00:27 +0000
+++ lib/lp/buildmaster/model/buildfarmjobbehavior.py	2013-01-22 09:25:35 +0000
@@ -98,10 +98,10 @@
         return d
 
     @defer.inlineCallbacks
-    def storeLogFromSlave(self):
+    def storeLogFromSlave(self, build_queue=None):
         """See `IBuildFarmJob`."""
         lfa_id = yield self.getLogFromSlave(
-            self.build, self.build.buildqueue_record)
+            self.build, build_queue or self.build.buildqueue_record)
         self.build.setLog(lfa_id)
 
     def updateBuild(self, queueItem):

=== modified file 'lib/lp/translations/configure.zcml'
--- lib/lp/translations/configure.zcml	2012-02-15 21:14:05 +0000
+++ lib/lp/translations/configure.zcml	2013-01-22 09:25:35 +0000
@@ -632,12 +632,6 @@
     <class
         class="lp.translations.model.translationtemplatesbuild.TranslationTemplatesBuild">
         <require permission="launchpad.View" interface="lp.translations.interfaces.translationtemplatesbuild.ITranslationTemplatesBuild"/>
-        <!-- This is needed for BuildManager to run. The permission isn't
-             important; launchpad.Edit isn't actually held by anybody.
-             Inspired by the similar change for SourcePackageRecipeBuild. -->
-        <require permission="launchpad.Edit"
-            set_attributes="builder date_finished date_started log status" />
-
     </class>
     <securedutility
         component="lp.translations.model.translationtemplatesbuild.TranslationTemplatesBuild"

=== modified file 'lib/lp/translations/model/translationtemplatesbuildbehavior.py'
--- lib/lp/translations/model/translationtemplatesbuildbehavior.py	2013-01-09 07:58:55 +0000
+++ lib/lp/translations/model/translationtemplatesbuildbehavior.py	2013-01-22 09:25:35 +0000
@@ -11,11 +11,9 @@
     'TranslationTemplatesBuildBehavior',
     ]
 
-import datetime
 import os
 import tempfile
 
-import pytz
 from twisted.internet import defer
 from zope.component import getUtility
 from zope.interface import implements
@@ -119,24 +117,7 @@
             if len(raw_slave_status) >= 4:
                 status['filemap'] = raw_slave_status[3]
 
-    def setBuildStatus(self, status):
-        self.build.status = status
-
-    @classmethod
-    def storeBuildInfo(cls, build, queue_item, build_status):
-        """See `IPackageBuild`."""
-        def got_log(lfa_id):
-            build.log = lfa_id
-            build.builder = queue_item.builder
-            build.date_started = queue_item.date_started
-            # XXX cprov 20060615 bug=120584: Currently buildduration includes
-            # the scanner latency, it should really be asking the slave for
-            # the duration spent building locally.
-            build.date_finished = datetime.datetime.now(pytz.UTC)
-
-        d = cls.getLogFromSlave(build, queue_item)
-        return d.addCallback(got_log)
-
+    @defer.inlineCallbacks
     def updateBuild_WAITING(self, queue_item, slave_status, logtail, logger):
         """Deal with a finished ("WAITING" state, perversely) build job.
 
@@ -154,48 +135,41 @@
             queue_item.specific_job.branch.bzr_identity,
             build_status))
 
-        def clean_slave(ignored):
-            d = queue_item.builder.cleanSlave()
-            return d.addCallback(lambda ignored: queue_item.destroySelf())
+        if build_status == 'OK':
+            self.build.updateStatus(
+                BuildStatus.UPLOADING, builder=queue_item.builder)
+            logger.debug("Processing successful templates build.")
+            filemap = slave_status.get('filemap')
+            filename = yield self._readTarball(queue_item, filemap, logger)
 
-        def got_tarball(filename):
             # XXX 2010-11-12 bug=674575
             # Please make addOrUpdateEntriesFromTarball() take files on
             # disk; reading arbitrarily sized files into memory is
             # dangerous.
             if filename is None:
                 logger.error("Build produced no tarball.")
-                self.setBuildStatus(BuildStatus.FULLYBUILT)
-                return
-
-            tarball_file = open(filename)
-            try:
-                tarball = tarball_file.read()
-                if tarball is None:
-                    logger.error("Build produced empty tarball.")
-                else:
-                    logger.debug("Uploading translation templates tarball.")
-                    self._uploadTarball(
-                        queue_item.specific_job.branch, tarball, logger)
-                    logger.debug("Upload complete.")
-            finally:
-                self.setBuildStatus(BuildStatus.FULLYBUILT)
-                tarball_file.close()
-                os.remove(filename)
-
-        def build_info_stored(ignored):
-            if build_status == 'OK':
-                self.setBuildStatus(BuildStatus.UPLOADING)
-                logger.debug("Processing successful templates build.")
-                filemap = slave_status.get('filemap')
-                d = self._readTarball(queue_item, filemap, logger)
-                d.addCallback(got_tarball)
-                d.addCallback(clean_slave)
-                return d
-
-            self.setBuildStatus(BuildStatus.FAILEDTOBUILD)
-            return clean_slave(None)
-
-        d = self.storeBuildInfo(self.build, queue_item, build_status)
-        d.addCallback(build_info_stored)
-        return d
+                self.build.updateStatus(BuildStatus.FULLYBUILT)
+            else:
+                tarball_file = open(filename)
+                try:
+                    tarball = tarball_file.read()
+                    if tarball is None:
+                        logger.error("Build produced empty tarball.")
+                    else:
+                        logger.debug(
+                            "Uploading translation templates tarball.")
+                        self._uploadTarball(
+                            queue_item.specific_job.branch, tarball, logger)
+                        logger.debug("Upload complete.")
+                finally:
+                    self.build.updateStatus(BuildStatus.FULLYBUILT)
+                    tarball_file.close()
+                    os.remove(filename)
+        else:
+            self.build.updateStatus(
+                BuildStatus.FAILEDTOBUILD, builder=queue_item.builder)
+
+        yield self.storeLogFromSlave(build_queue=queue_item)
+
+        yield queue_item.builder.cleanSlave()
+        queue_item.destroySelf()