Merge lp:~stub/launchpad/cronscripts into lp:launchpad

[Stuart Bishop]

Load the cronscript control file from a URL

The cronscript control file will be more useful if it can exist in one location rather than a separate copy on the dozen or so servers we run cronscripts on. So lets not use a filesystem path and instead use a URL. We can always use file: urls if we do want to use the local file system, which is how the tests work.

[Robert Collins (lifeless)]

A few thoughts:
 - needs an os.path.abspath() around the tempfile name - /tmp is only the default, but you're making a url out of it.
 - probably want a timeout on the file grabbing.
 - if reading the file fails, perhaps we should fail-closed?

review: Approve

=== modified file 'configs/testrunner/launchpad-lazr.conf'
--- configs/testrunner/launchpad-lazr.conf	2010-08-06 09:01:56 +0000
+++ configs/testrunner/launchpad-lazr.conf	2010-09-17 04:48:55 +0000
@@ -7,7 +7,7 @@
 
 [canonical]
 chunkydiff: False
-cron_control_file: lib/lp/services/scripts/tests/cronscripts.ini
+cron_control_url: file:lib/lp/services/scripts/tests/cronscripts.ini
 
 [archivepublisher]
 base_url: http://ftpmaster.internal/

=== modified file 'lib/canonical/config/schema-lazr.conf'
--- lib/canonical/config/schema-lazr.conf	2010-09-09 02:26:42 +0000
+++ lib/canonical/config/schema-lazr.conf	2010-09-17 04:48:55 +0000
@@ -210,9 +210,8 @@
 # datatype: string
 admin_address: [e-mail]
 
-# By default, relative to the root.
-# datatype: filename
-cron_control_file: cronscripts.ini
+# datatype: urlbase
+cron_control_url: file:cronscripts.ini
 
 
 [checkwatches]

=== modified file 'lib/lp/services/scripts/base.py'
--- lib/lp/services/scripts/base.py	2010-08-20 20:31:18 +0000
+++ lib/lp/services/scripts/base.py	2010-09-17 04:48:55 +0000
@@ -6,7 +6,7 @@
     'LaunchpadCronScript',
     'LaunchpadScript',
     'LaunchpadScriptFailure',
-    'SilentLaunchpadScriptFailure'
+    'SilentLaunchpadScriptFailure',
     ]
 
 from ConfigParser import SafeConfigParser
@@ -16,6 +16,7 @@
 from optparse import OptionParser
 import os.path
 import sys
+from urllib2 import urlopen, HTTPError, URLError
 
 from contrib.glock import (
     GlobalLock,
@@ -59,6 +60,7 @@
 
 class SilentLaunchpadScriptFailure(Exception):
     """A LaunchpadScriptFailure that doesn't log an error."""
+
     def __init__(self, exit_status=1):
         Exception.__init__(self, exit_status)
         self.exit_status = exit_status
@@ -145,7 +147,6 @@
     #
     # Hooks that we expect users to redefine.
     #
-
     def main(self):
         """Define the meat of your script here. Must be defined.
 
@@ -169,7 +170,6 @@
     #
     # Convenience or death
     #
-
     def login(self, user):
         """Super-convenience method that avoids the import."""
         setupInteractionByEmail(user)
@@ -179,7 +179,6 @@
     # they really want to control the run-and-locking semantics of the
     # script carefully.
     #
-
     @property
     def lockfilename(self):
         """Return lockfilename.
@@ -278,7 +277,6 @@
     #
     # Make things happen
     #
-
     def lock_and_run(self, blocking=False, skip_delete=False,
                      use_web_security=False, implicit_begin=True,
                      isolation=ISOLATION_LEVEL_DEFAULT):
@@ -304,12 +302,12 @@
         enabled and disabled using a config file.
 
         The control file location is specified by
-        config.canonical.cron_control_file.
+        config.canonical.cron_control_url.
         """
         super(LaunchpadCronScript, self).__init__(name, dbuser, test_args)
 
         enabled = cronscript_enabled(
-            config.canonical.cron_control_file, self.name, self.logger)
+            config.canonical.cron_control_url, self.name, self.logger)
         if not enabled:
             sys.exit(0)
 
@@ -324,26 +322,40 @@
         self.txn.commit()
 
 
-def cronscript_enabled(control_path, name, log):
+def cronscript_enabled(control_url, name, log):
     """Return True if the cronscript is enabled."""
-    if not os.path.isabs(control_path):
-        control_path = os.path.abspath(
-            os.path.join(config.root, control_path))
+    try:
+        if sys.version_info[:2] >= (2, 6):
+            # Timeout of 5 seconds should be fine on the LAN. We don't want
+            # the default as it is too long for scripts being run every 60
+            # seconds.
+            control_fp = urlopen(control_url, timeout=5)
+        else:
+            control_fp = urlopen(control_url)
+    except HTTPError, error:
+        if error.code == 404:
+            log.debug("Cronscript control file not found at %s", control_url)
+            return True
+        log.exception("Error loading %s" % control_url)
+        return True
+    except URLError, error:
+        if getattr(error.reason, 'errno', None) == 2:
+            log.debug("Cronscript control file not found at %s", control_url)
+            return True
+        log.exception("Error loading %s" % control_url)
+        return True
+    except:
+        log.exception("Error loading %s" % control_url)
+        return True
 
     cron_config = SafeConfigParser({'enabled': str(True)})
 
-    if not os.path.exists(control_path):
-        # No control file exists. Everything enabled by default.
-        log.debug("Cronscript control file not found at %s", control_path)
-    else:
-        log.debug("Cronscript control file found at %s", control_path)
-
-        # Try reading the config file. If it fails, we log the
-        # traceback and continue on using the defaults.
-        try:
-            cron_config.read(control_path)
-        except:
-            log.exception("Error parsing %s", control_path)
+    # Try reading the config file. If it fails, we log the
+    # traceback and continue on using the defaults.
+    try:
+        cron_config.readfp(control_fp)
+    except:
+        log.exception("Error parsing %s", control_url)
 
     if cron_config.has_option(name, 'enabled'):
         section = name
@@ -355,7 +367,7 @@
     except:
         log.exception(
             "Failed to load value from %s section of %s",
-            section, control_path)
+            section, control_url)
         enabled = True
 
     if enabled:

=== modified file 'lib/lp/services/scripts/tests/test_cronscript_enabled.py'
--- lib/lp/services/scripts/tests/test_cronscript_enabled.py	2010-08-06 09:01:56 +0000
+++ lib/lp/services/scripts/tests/test_cronscript_enabled.py	2010-09-17 04:48:55 +0000
@@ -20,6 +20,7 @@
 
 
 class TestCronscriptEnabled(TestCase):
+
     def setUp(self):
         super(TestCronscriptEnabled, self).setUp()
         self.log_output = StringIO()
@@ -33,10 +34,10 @@
         # Ensure a reference is kept until the test is over.
         # tempfile will then clean itself up.
         self.addCleanup(lambda x: None, tempfile)
-        return tempfile.name
+        return 'file:' + os.path.abspath(tempfile.name)
 
     def test_noconfig(self):
-        enabled = cronscript_enabled('/idontexist.ini', 'foo', self.log)
+        enabled = cronscript_enabled('file:/idontexist.ini', 'foo', self.log)
         self.assertIs(True, enabled)
 
     def test_emptyconfig(self):