launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/publish-ftpmaster-cron-control into lp:launchpad.

Commit message:
Don't allow cron-control to interrupt publish-ftpmaster part-way through.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1647478 in Launchpad itself: "publish-ftpmaster allows its publish-distro calls to be interrupted by cron-control"
  https://bugs.launchpad.net/launchpad/+bug/1647478

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/publish-ftpmaster-cron-control/+merge/312511
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/publish-ftpmaster-cron-control into lp:launchpad.

=== modified file 'lib/lp/archivepublisher/scripts/publish_ftpmaster.py'
--- lib/lp/archivepublisher/scripts/publish_ftpmaster.py	2016-11-11 15:31:08 +0000
+++ lib/lp/archivepublisher/scripts/publish_ftpmaster.py	2016-12-05 22:20:07 +0000
@@ -334,7 +334,8 @@
         self.logger.debug(
             "Processing the accepted queue into the publishing records...")
         script = ProcessAccepted(
-            test_args=["-d", distribution.name], logger=self.logger)
+            test_args=["-d", distribution.name], logger=self.logger,
+            ignore_cron_control=True)
         script.txn = self.txn
         script.main()
 
@@ -428,7 +429,7 @@
             sum([['-s', suite] for suite in suites], []))
 
         publish_distro = PublishDistro(
-            test_args=arguments, logger=self.logger)
+            test_args=arguments, logger=self.logger, ignore_cron_control=True)
         publish_distro.logger = self.logger
         publish_distro.txn = self.txn
         publish_distro.main()

=== modified file 'lib/lp/archivepublisher/tests/test_publish_ftpmaster.py'
--- lib/lp/archivepublisher/tests/test_publish_ftpmaster.py	2016-11-07 16:42:23 +0000
+++ lib/lp/archivepublisher/tests/test_publish_ftpmaster.py	2016-12-05 22:20:07 +0000
@@ -11,10 +11,13 @@
 import time
 
 from apt_pkg import TagFile
+from fixtures import MonkeyPatch
 from testtools.matchers import (
+    MatchesException,
     MatchesStructure,
     Not,
     PathExists,
+    Raises,
     StartsWith,
     )
 from zope.component import getUtility
@@ -1080,6 +1083,22 @@
 
         self.assertEqual(1, script.recoverArchiveWorkingDir.call_count)
 
+    def test_publish_is_not_interrupted_by_cron_control(self):
+        # If cron-control switches to the disabled state in the middle of a
+        # publisher run, all the subsidiary scripts are still run.
+        script = self.makeScript()
+        self.useFixture(MonkeyPatch(
+            "lp.services.scripts.base.cronscript_enabled", FakeMethod(False)))
+        process_accepted_fixture = self.useFixture(MonkeyPatch(
+            "lp.archivepublisher.scripts.processaccepted.ProcessAccepted.main",
+            FakeMethod()))
+        publish_distro_fixture = self.useFixture(MonkeyPatch(
+            "lp.archivepublisher.scripts.publishdistro.PublishDistro.main",
+            FakeMethod()))
+        self.assertThat(script.main, Not(Raises(MatchesException(SystemExit))))
+        self.assertEqual(1, process_accepted_fixture.new_value.call_count)
+        self.assertEqual(1, publish_distro_fixture.new_value.call_count)
+
 
 class TestCreateDistroSeriesIndexes(TestCaseWithFactory, HelpersMixin):
     """Test initial creation of archive indexes for a `DistroSeries`."""

=== modified file 'lib/lp/services/scripts/base.py'
--- lib/lp/services/scripts/base.py	2014-08-29 01:34:04 +0000
+++ lib/lp/services/scripts/base.py	2016-12-05 22:20:07 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -376,17 +376,19 @@
 class LaunchpadCronScript(LaunchpadScript):
     """Logs successful script runs in the database."""
 
-    def __init__(self, name=None, dbuser=None, test_args=None, logger=None):
+    def __init__(self, name=None, dbuser=None, test_args=None, logger=None,
+                 ignore_cron_control=False):
         super(LaunchpadCronScript, self).__init__(
             name, dbuser, test_args=test_args, logger=logger)
 
         # self.name is used instead of the name argument, since it may have
         # have been overridden by command-line parameters or by
         # overriding the name property.
-        enabled = cronscript_enabled(
-            config.canonical.cron_control_url, self.name, self.logger)
-        if not enabled:
-            sys.exit(0)
+        if not ignore_cron_control:
+            enabled = cronscript_enabled(
+                config.canonical.cron_control_url, self.name, self.logger)
+            if not enabled:
+                sys.exit(0)
 
         # Configure the IErrorReportingUtility we use with defaults.
         # Scripts can override this if they want.