launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/cancelled-buildlog into lp:launchpad with lp:~cjwatson/launchpad/buildmaster-cancel-properly as a prerequisite.

Commit message:
Retrieve build log when a build is cancelled.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1050514 in Launchpad itself: "Cancelling build loses log file"
  https://bugs.launchpad.net/launchpad/+bug/1050514

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/cancelled-buildlog/+merge/182883

Following buildmaster-cancel-properly, we're almost ready to start auto-cancelling superseded builds, so that people can't clog up the build farm with a sequence of builds all but one of which will fail to upload.  However, before we start auto-cancelling things, it would be friendly to ensure we don't lose the build log when doing so.

I've only implemented this for the case of a deliberate cancel, because who knows what the builder state is otherwise.  If you think we need to care about this for other WAITING/ABORTED cases, it's easy enough to change.
-- 
https://code.launchpad.net/~cjwatson/launchpad/cancelled-buildlog/+merge/182883
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/cancelled-buildlog into lp:launchpad.

=== modified file 'lib/lp/buildmaster/model/buildfarmjobbehavior.py'
--- lib/lp/buildmaster/model/buildfarmjobbehavior.py	2013-08-29 05:05:06 +0000
+++ lib/lp/buildmaster/model/buildfarmjobbehavior.py	2013-08-29 11:52:13 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2010 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Base and idle BuildFarmJobBehavior classes."""
@@ -320,6 +320,7 @@
         Otherwise, the build has failed in some unexpected way.
         """
         if self.build.status == BuildStatus.CANCELLING:
+            yield self.storeLogFromSlave()
             self.build.buildqueue_record.cancel()
             transaction.commit()
             yield self._interactor.cleanSlave()

=== modified file 'lib/lp/buildmaster/tests/test_buildfarmjobbehavior.py'
--- lib/lp/buildmaster/tests/test_buildfarmjobbehavior.py	2013-08-29 04:58:10 +0000
+++ lib/lp/buildmaster/tests/test_buildfarmjobbehavior.py	2013-08-29 11:52:13 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Unit tests for BuildFarmJobBehaviorBase."""
@@ -10,6 +10,7 @@
 import shutil
 import tempfile
 
+from twisted.internet import defer
 from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
 
@@ -302,6 +303,14 @@
         d = self.behavior.handleStatus("ABORTED", {})
         return d.addCallback(got_status)
 
+    @defer.inlineCallbacks
+    def test_handleStatus_ABORTED_cancelling_sets_build_log(self):
+        # If a build is intentionally cancelled, the build log is set.
+        self.assertEqual(None, self.build.log)
+        self.build.updateStatus(BuildStatus.CANCELLING)
+        yield self.behavior.handleStatus("ABORTED", None, {})
+        self.assertNotEqual(None, self.build.log)
+
     def test_date_finished_set(self):
         # The date finished is updated during handleStatus_OK.
         self.assertEqual(None, self.build.date_finished)