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
1=== modified file 'configs/testrunner/launchpad-lazr.conf'
2--- configs/testrunner/launchpad-lazr.conf 2010-08-06 09:01:56 +0000
3+++ configs/testrunner/launchpad-lazr.conf 2010-09-17 04:48:55 +0000
4@@ -7,7 +7,7 @@
5
6 [canonical]
7 chunkydiff: False
8-cron_control_file: lib/lp/services/scripts/tests/cronscripts.ini
9+cron_control_url: file:lib/lp/services/scripts/tests/cronscripts.ini
10
11 [archivepublisher]
12 base_url: http://ftpmaster.internal/
13
14=== modified file 'lib/canonical/config/schema-lazr.conf'
15--- lib/canonical/config/schema-lazr.conf 2010-09-09 02:26:42 +0000
16+++ lib/canonical/config/schema-lazr.conf 2010-09-17 04:48:55 +0000
17@@ -210,9 +210,8 @@
18 # datatype: string
19 admin_address: system-error@launchpad.net
20
21-# By default, relative to the root.
22-# datatype: filename
23-cron_control_file: cronscripts.ini
24+# datatype: urlbase
25+cron_control_url: file:cronscripts.ini
26
27
28 [checkwatches]
29
30=== modified file 'lib/lp/services/scripts/base.py'
31--- lib/lp/services/scripts/base.py 2010-08-20 20:31:18 +0000
32+++ lib/lp/services/scripts/base.py 2010-09-17 04:48:55 +0000
33@@ -6,7 +6,7 @@
34 'LaunchpadCronScript',
35 'LaunchpadScript',
36 'LaunchpadScriptFailure',
37- 'SilentLaunchpadScriptFailure'
38+ 'SilentLaunchpadScriptFailure',
39 ]
40
41 from ConfigParser import SafeConfigParser
42@@ -16,6 +16,7 @@
43 from optparse import OptionParser
44 import os.path
45 import sys
46+from urllib2 import urlopen, HTTPError, URLError
47
48 from contrib.glock import (
49 GlobalLock,
50@@ -59,6 +60,7 @@
51
52 class SilentLaunchpadScriptFailure(Exception):
53 """A LaunchpadScriptFailure that doesn't log an error."""
54+
55 def __init__(self, exit_status=1):
56 Exception.__init__(self, exit_status)
57 self.exit_status = exit_status
58@@ -145,7 +147,6 @@
59 #
60 # Hooks that we expect users to redefine.
61 #
62-
63 def main(self):
64 """Define the meat of your script here. Must be defined.
65
66@@ -169,7 +170,6 @@
67 #
68 # Convenience or death
69 #
70-
71 def login(self, user):
72 """Super-convenience method that avoids the import."""
73 setupInteractionByEmail(user)
74@@ -179,7 +179,6 @@
75 # they really want to control the run-and-locking semantics of the
76 # script carefully.
77 #
78-
79 @property
80 def lockfilename(self):
81 """Return lockfilename.
82@@ -278,7 +277,6 @@
83 #
84 # Make things happen
85 #
86-
87 def lock_and_run(self, blocking=False, skip_delete=False,
88 use_web_security=False, implicit_begin=True,
89 isolation=ISOLATION_LEVEL_DEFAULT):
90@@ -304,12 +302,12 @@
91 enabled and disabled using a config file.
92
93 The control file location is specified by
94- config.canonical.cron_control_file.
95+ config.canonical.cron_control_url.
96 """
97 super(LaunchpadCronScript, self).__init__(name, dbuser, test_args)
98
99 enabled = cronscript_enabled(
100- config.canonical.cron_control_file, self.name, self.logger)
101+ config.canonical.cron_control_url, self.name, self.logger)
102 if not enabled:
103 sys.exit(0)
104
105@@ -324,26 +322,40 @@
106 self.txn.commit()
107
108
109-def cronscript_enabled(control_path, name, log):
110+def cronscript_enabled(control_url, name, log):
111 """Return True if the cronscript is enabled."""
112- if not os.path.isabs(control_path):
113- control_path = os.path.abspath(
114- os.path.join(config.root, control_path))
115+ try:
116+ if sys.version_info[:2] >= (2, 6):
117+ # Timeout of 5 seconds should be fine on the LAN. We don't want
118+ # the default as it is too long for scripts being run every 60
119+ # seconds.
120+ control_fp = urlopen(control_url, timeout=5)
121+ else:
122+ control_fp = urlopen(control_url)
123+ except HTTPError, error:
124+ if error.code == 404:
125+ log.debug("Cronscript control file not found at %s", control_url)
126+ return True
127+ log.exception("Error loading %s" % control_url)
128+ return True
129+ except URLError, error:
130+ if getattr(error.reason, 'errno', None) == 2:
131+ log.debug("Cronscript control file not found at %s", control_url)
132+ return True
133+ log.exception("Error loading %s" % control_url)
134+ return True
135+ except:
136+ log.exception("Error loading %s" % control_url)
137+ return True
138
139 cron_config = SafeConfigParser({'enabled': str(True)})
140
141- if not os.path.exists(control_path):
142- # No control file exists. Everything enabled by default.
143- log.debug("Cronscript control file not found at %s", control_path)
144- else:
145- log.debug("Cronscript control file found at %s", control_path)
146-
147- # Try reading the config file. If it fails, we log the
148- # traceback and continue on using the defaults.
149- try:
150- cron_config.read(control_path)
151- except:
152- log.exception("Error parsing %s", control_path)
153+ # Try reading the config file. If it fails, we log the
154+ # traceback and continue on using the defaults.
155+ try:
156+ cron_config.readfp(control_fp)
157+ except:
158+ log.exception("Error parsing %s", control_url)
159
160 if cron_config.has_option(name, 'enabled'):
161 section = name
162@@ -355,7 +367,7 @@
163 except:
164 log.exception(
165 "Failed to load value from %s section of %s",
166- section, control_path)
167+ section, control_url)
168 enabled = True
169
170 if enabled:
171
172=== modified file 'lib/lp/services/scripts/tests/test_cronscript_enabled.py'
173--- lib/lp/services/scripts/tests/test_cronscript_enabled.py 2010-08-06 09:01:56 +0000
174+++ lib/lp/services/scripts/tests/test_cronscript_enabled.py 2010-09-17 04:48:55 +0000
175@@ -20,6 +20,7 @@
176
177
178 class TestCronscriptEnabled(TestCase):
179+
180 def setUp(self):
181 super(TestCronscriptEnabled, self).setUp()
182 self.log_output = StringIO()
183@@ -33,10 +34,10 @@
184 # Ensure a reference is kept until the test is over.
185 # tempfile will then clean itself up.
186 self.addCleanup(lambda x: None, tempfile)
187- return tempfile.name
188+ return 'file:' + os.path.abspath(tempfile.name)
189
190 def test_noconfig(self):
191- enabled = cronscript_enabled('/idontexist.ini', 'foo', self.log)
192+ enabled = cronscript_enabled('file:/idontexist.ini', 'foo', self.log)
193 self.assertIs(True, enabled)
194
195 def test_emptyconfig(self):