Launchpad itself

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

  1. cronscripts
  2. Merge into devel
Proposed by Stuart Bishop
Status: Merged
Approved by: Robert Collins
Approved revision: no longer in the source branch.
Merged at revision: 11567
Proposed branch: lp:~stub/launchpad/cronscripts
Merge into: lp:launchpad
Diff against target: 195 lines (+42/-30)
4 files modified
configs/testrunner/launchpad-lazr.conf (+1/-1)
lib/canonical/config/schema-lazr.conf (+2/-3)
lib/lp/services/scripts/base.py (+36/-24)
lib/lp/services/scripts/tests/test_cronscript_enabled.py (+3/-2)
To merge this branch: bzr merge lp:~stub/launchpad/cronscripts
Reviewer Review Type Date Requested Status
Robert Collins (community) Approve
Review via email: mp+35279@code.launchpad.net

Commit message

Load the cronscript control file from a URL

Description of the change

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.

To post a comment you must log in.
Revision history for this message
Robert Collins (lifeless) wrote :

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
77
8[canonical]8[canonical]
9chunkydiff: False9chunkydiff: False
10cron_control_file: lib/lp/services/scripts/tests/cronscripts.ini10cron_control_url: file:lib/lp/services/scripts/tests/cronscripts.ini
1111
12[archivepublisher]12[archivepublisher]
13base_url: http://ftpmaster.internal/13base_url: http://ftpmaster.internal/
1414
=== 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 @@
210# datatype: string210# datatype: string
211admin_address: system-error@launchpad.net211admin_address: system-error@launchpad.net
212212
213# By default, relative to the root.213# datatype: urlbase
214# datatype: filename214cron_control_url: file:cronscripts.ini
215cron_control_file: cronscripts.ini
216215
217216
218[checkwatches]217[checkwatches]
219218
=== 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 @@
6 'LaunchpadCronScript',6 'LaunchpadCronScript',
7 'LaunchpadScript',7 'LaunchpadScript',
8 'LaunchpadScriptFailure',8 'LaunchpadScriptFailure',
9 'SilentLaunchpadScriptFailure'9 'SilentLaunchpadScriptFailure',
10 ]10 ]
1111
12from ConfigParser import SafeConfigParser12from ConfigParser import SafeConfigParser
@@ -16,6 +16,7 @@
16from optparse import OptionParser16from optparse import OptionParser
17import os.path17import os.path
18import sys18import sys
19from urllib2 import urlopen, HTTPError, URLError
1920
20from contrib.glock import (21from contrib.glock import (
21 GlobalLock,22 GlobalLock,
@@ -59,6 +60,7 @@
5960
60class SilentLaunchpadScriptFailure(Exception):61class SilentLaunchpadScriptFailure(Exception):
61 """A LaunchpadScriptFailure that doesn't log an error."""62 """A LaunchpadScriptFailure that doesn't log an error."""
63
62 def __init__(self, exit_status=1):64 def __init__(self, exit_status=1):
63 Exception.__init__(self, exit_status)65 Exception.__init__(self, exit_status)
64 self.exit_status = exit_status66 self.exit_status = exit_status
@@ -145,7 +147,6 @@
145 #147 #
146 # Hooks that we expect users to redefine.148 # Hooks that we expect users to redefine.
147 #149 #
148
149 def main(self):150 def main(self):
150 """Define the meat of your script here. Must be defined.151 """Define the meat of your script here. Must be defined.
151152
@@ -169,7 +170,6 @@
169 #170 #
170 # Convenience or death171 # Convenience or death
171 #172 #
172
173 def login(self, user):173 def login(self, user):
174 """Super-convenience method that avoids the import."""174 """Super-convenience method that avoids the import."""
175 setupInteractionByEmail(user)175 setupInteractionByEmail(user)
@@ -179,7 +179,6 @@
179 # they really want to control the run-and-locking semantics of the179 # they really want to control the run-and-locking semantics of the
180 # script carefully.180 # script carefully.
181 #181 #
182
183 @property182 @property
184 def lockfilename(self):183 def lockfilename(self):
185 """Return lockfilename.184 """Return lockfilename.
@@ -278,7 +277,6 @@
278 #277 #
279 # Make things happen278 # Make things happen
280 #279 #
281
282 def lock_and_run(self, blocking=False, skip_delete=False,280 def lock_and_run(self, blocking=False, skip_delete=False,
283 use_web_security=False, implicit_begin=True,281 use_web_security=False, implicit_begin=True,
284 isolation=ISOLATION_LEVEL_DEFAULT):282 isolation=ISOLATION_LEVEL_DEFAULT):
@@ -304,12 +302,12 @@
304 enabled and disabled using a config file.302 enabled and disabled using a config file.
305303
306 The control file location is specified by304 The control file location is specified by
307 config.canonical.cron_control_file.305 config.canonical.cron_control_url.
308 """306 """
309 super(LaunchpadCronScript, self).__init__(name, dbuser, test_args)307 super(LaunchpadCronScript, self).__init__(name, dbuser, test_args)
310308
311 enabled = cronscript_enabled(309 enabled = cronscript_enabled(
312 config.canonical.cron_control_file, self.name, self.logger)310 config.canonical.cron_control_url, self.name, self.logger)
313 if not enabled:311 if not enabled:
314 sys.exit(0)312 sys.exit(0)
315313
@@ -324,26 +322,40 @@
324 self.txn.commit()322 self.txn.commit()
325323
326324
327def cronscript_enabled(control_path, name, log):325def cronscript_enabled(control_url, name, log):
328 """Return True if the cronscript is enabled."""326 """Return True if the cronscript is enabled."""
329 if not os.path.isabs(control_path):327 try:
330 control_path = os.path.abspath(328 if sys.version_info[:2] >= (2, 6):
331 os.path.join(config.root, control_path))329 # Timeout of 5 seconds should be fine on the LAN. We don't want
330 # the default as it is too long for scripts being run every 60
331 # seconds.
332 control_fp = urlopen(control_url, timeout=5)
333 else:
334 control_fp = urlopen(control_url)
335 except HTTPError, error:
336 if error.code == 404:
337 log.debug("Cronscript control file not found at %s", control_url)
338 return True
339 log.exception("Error loading %s" % control_url)
340 return True
341 except URLError, error:
342 if getattr(error.reason, 'errno', None) == 2:
343 log.debug("Cronscript control file not found at %s", control_url)
344 return True
345 log.exception("Error loading %s" % control_url)
346 return True
347 except:
348 log.exception("Error loading %s" % control_url)
349 return True
332350
333 cron_config = SafeConfigParser({'enabled': str(True)})351 cron_config = SafeConfigParser({'enabled': str(True)})
334352
335 if not os.path.exists(control_path):353 # Try reading the config file. If it fails, we log the
336 # No control file exists. Everything enabled by default.354 # traceback and continue on using the defaults.
337 log.debug("Cronscript control file not found at %s", control_path)355 try:
338 else:356 cron_config.readfp(control_fp)
339 log.debug("Cronscript control file found at %s", control_path)357 except:
340358 log.exception("Error parsing %s", control_url)
341 # Try reading the config file. If it fails, we log the
342 # traceback and continue on using the defaults.
343 try:
344 cron_config.read(control_path)
345 except:
346 log.exception("Error parsing %s", control_path)
347359
348 if cron_config.has_option(name, 'enabled'):360 if cron_config.has_option(name, 'enabled'):
349 section = name361 section = name
@@ -355,7 +367,7 @@
355 except:367 except:
356 log.exception(368 log.exception(
357 "Failed to load value from %s section of %s",369 "Failed to load value from %s section of %s",
358 section, control_path)370 section, control_url)
359 enabled = True371 enabled = True
360372
361 if enabled:373 if enabled:
362374
=== 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 @@
2020
2121
22class TestCronscriptEnabled(TestCase):22class TestCronscriptEnabled(TestCase):
23
23 def setUp(self):24 def setUp(self):
24 super(TestCronscriptEnabled, self).setUp()25 super(TestCronscriptEnabled, self).setUp()
25 self.log_output = StringIO()26 self.log_output = StringIO()
@@ -33,10 +34,10 @@
33 # Ensure a reference is kept until the test is over.34 # Ensure a reference is kept until the test is over.
34 # tempfile will then clean itself up.35 # tempfile will then clean itself up.
35 self.addCleanup(lambda x: None, tempfile)36 self.addCleanup(lambda x: None, tempfile)
36 return tempfile.name37 return 'file:' + os.path.abspath(tempfile.name)
3738
38 def test_noconfig(self):39 def test_noconfig(self):
39 enabled = cronscript_enabled('/idontexist.ini', 'foo', self.log)40 enabled = cronscript_enabled('file:/idontexist.ini', 'foo', self.log)
40 self.assertIs(True, enabled)41 self.assertIs(True, enabled)
4142
42 def test_emptyconfig(self):43 def test_emptyconfig(self):