Merge lp:~adeuring/launchpad/lp-lazr.jobrunner into lp:launchpad
- lp-lazr.jobrunner
- Merge into devel
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Aaron Bentley | ||||
Approved revision: | no longer in the source branch. | ||||
Merged at revision: | 15037 | ||||
Proposed branch: | lp:~adeuring/launchpad/lp-lazr.jobrunner | ||||
Merge into: | lp:launchpad | ||||
Diff against target: |
606 lines (+98/-136) 15 files modified
lib/lp/code/scripts/tests/test_create_merge_proposals.py (+3/-3) lib/lp/code/scripts/tests/test_merge_proposal_jobs.py (+2/-1) lib/lp/code/scripts/tests/test_reclaim_branch_space.py (+7/-5) lib/lp/code/scripts/tests/test_sendbranchmail.py (+12/-7) lib/lp/registry/tests/test_process_job_sources_cronjob.py (+2/-2) lib/lp/services/job/interfaces/job.py (+3/-6) lib/lp/services/job/model/job.py (+28/-5) lib/lp/services/job/runner.py (+17/-89) lib/lp/services/job/tests/test_runner.py (+11/-12) lib/lp/soyuz/model/packagecopyjob.py (+1/-1) lib/lp/soyuz/tests/test_packagecopyjob.py (+1/-1) lib/lp/testing/factory.py (+1/-1) lib/lp/translations/scripts/tests/test_packaging_translations.py (+2/-2) setup.py (+1/-0) versions.cfg (+7/-1) |
||||
To merge this branch: | bzr merge lp:~adeuring/launchpad/lp-lazr.jobrunner | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Aaron Bentley (community) | Approve | ||
Abel Deuring (community) | Needs Resubmitting | ||
Review via email: mp+97458@code.launchpad.net |
Commit message
use lazr.jobrunner for basic job management
Description of the change
This branch removes some core methods from class JobRunner, which
are now provided by lazr.jobrunner.
It requires lp:~adeuring/launchpad/lazr.jobrunner-more-tests (not yet
merged into lp:lazr.jobrunner/trunk)
lazr.jobrunner does not do everything which the old job runner did.
Most notably, the now removed method BaseJobRunner.
several calls of transaction.
The new module lazr.jobrunner does make any assumption where or how
the status of a job is stored, or if a job makes any other changes
to a database, and hence does do any of these calls.
Instead, these calls are now done in Job.start(), Job.complete() etc.
Grepping through the Launchpad source code showed that some of these
methods are called in places that are not related to controlling a
job, but for example to create a job in the status "suspended".
Calling transaction.
"intermediate" commits; to avoid this, I added the parameter
manage_transaction to Job.start() etc, which is False by default,
but the calls in lazr.jobrunner use manage_
is somewhat ugly, but the alternative, keeping transaction management
in lazr.jobrunner, would also look odd: The module is supposed to do
job management, regardless of any possible database related activities
of a job.
test: ./bin/test -vvt lp.services.job
no lint
Abel Deuring (adeuring) wrote : | # |
> Not landable yet because it assumes lazr.jobrunner is a development egg.
> "../lazr.jobrunner" should not be listed in buildout.cfg, and a version
> of lazr.jobrunner should be specified in versions.cfg.
Done.
>
> runJob may not need to be overridden in BaseJobRunner. If not, please
> remove it.
I think that we should keep this method: class runner.
implements some required methods like notifyOops(), but it does not
implement start(), complete() etc. The latter methods are defined in
lp.services.
lazr.jobrunner.
def runJob(self, job):
in BaseJobRunner ensures that the Zope machinery adapts the "incomplete"
job to IRunnableJob. OK, IRunnableJob(job) is also called in
JobRunner.runAll() and in TwistedJobRunne
right now not strictly necessary in JobRunner.runJob() -- but keeping
JobRunner.runJob() makes it easier to remove runAll() later.
Alternatively, we could call IRunnableJob(job) in lazr.jobrunner, but I
am not sure if this makes sense.
> job_str may not need to be overridden in BaseJobRunner. If not, please
> remove it.
Right, this is no longer needed. Removed.
Aaron Bentley (abentley) wrote : | # |
> > runJob may not need to be overridden in BaseJobRunner. If not, please
> > remove it.
>
> I think that we should keep this method: class runner.
> implements some required methods like notifyOops(), but it does not
> implement start(), complete() etc. The latter methods are defined in
> lp.services.
> lazr.jobrunner.
>
> def runJob(self, job):
> super(BaseJobRu
>
> in BaseJobRunner ensures that the Zope machinery adapts the "incomplete"
> job to IRunnableJob. OK, IRunnableJob(job) is also called in
> JobRunner.runAll() and in TwistedJobRunne
> right now not strictly necessary in JobRunner.runJob() -- but keeping
> JobRunner.runJob() makes it easier to remove runAll() later.
I think this is fine for now. In the future, I think we might want to make the job source responsible for ensuring that the job is runnable, and have the runner assume the job is runnable.
> Alternatively, we could call IRunnableJob(job) in lazr.jobrunner, but I
> am not sure if this makes sense.
I'd like to keep zope.component out of lazr.jobrunner if we can.
Abel Deuring (adeuring) wrote : | # |
Some additional changes are necessary to use lazr.jobrunner (see also https:/
The change in lib/lp/
The second change: I added the method Job._doOops() again to BasJobRunner. The reason:
The implementation of _doOops() in lazr.jobrunner calls oops_report.
This conflicts with the way how Launchpad generates an OOPS in scripts: lp.services.
tests:
bin/test -vv \
-t lp.registry.
-t lp.registry.
-t lp.code.
-t lp.code.
-t lp.code.
-t lp.translations
-t lp.registry.
no lint
Abel Deuring (adeuring) : | # |
Aaron Bentley (abentley) wrote : | # |
I believe this would introduce a regression with database error handling. If there is a database error (like a permission failure) runJobHandleError will call Job.fail. But that will cause a database lookup. Which fails due to the previous error. e.g. http://
Abel Deuring (adeuring) wrote : | # |
The problem you mentioned is fixed in lazr.jobrunner (including tests).
Aaron Bentley (abentley) : | # |
Preview Diff
1 | === modified file 'lib/lp/code/scripts/tests/test_create_merge_proposals.py' |
2 | --- lib/lp/code/scripts/tests/test_create_merge_proposals.py 2012-01-01 02:58:52 +0000 |
3 | +++ lib/lp/code/scripts/tests/test_create_merge_proposals.py 2012-03-28 11:45:27 +0000 |
4 | @@ -36,11 +36,11 @@ |
5 | retcode, stdout, stderr = run_script( |
6 | 'cronscripts/create_merge_proposals.py', []) |
7 | self.assertEqual(0, retcode) |
8 | - self.assertEqual( |
9 | + self.assertTextMatchesExpressionIgnoreWhitespace( |
10 | "INFO Creating lockfile: " |
11 | "/var/lock/launchpad-create_merge_proposals.lock\n" |
12 | - "INFO " |
13 | - "Running CreateMergeProposalJob (ID %d) in status Waiting\n" |
14 | + "INFO Running <.*CreateMergeProposalJob object at .*?> " |
15 | + "\(ID %s\) in status Waiting\n" |
16 | "INFO Ran 1 CreateMergeProposalJobs.\n" % job.job.id, stderr) |
17 | self.assertEqual('', stdout) |
18 | self.assertEqual(1, source.landing_targets.count()) |
19 | |
20 | === modified file 'lib/lp/code/scripts/tests/test_merge_proposal_jobs.py' |
21 | --- lib/lp/code/scripts/tests/test_merge_proposal_jobs.py 2012-01-01 02:58:52 +0000 |
22 | +++ lib/lp/code/scripts/tests/test_merge_proposal_jobs.py 2012-03-28 11:45:27 +0000 |
23 | @@ -40,7 +40,8 @@ |
24 | '\tworkers: 0\n' |
25 | 'INFO \tworkers: 0\n' |
26 | '(.|\n)*' |
27 | - 'INFO Running GenerateIncrementalDiffJob \(ID %d\).\n' |
28 | + 'INFO Running ' |
29 | + '<GENERATE_INCREMENTAL_DIFF job for merge .*?> \(ID %d\).\n' |
30 | '(.|\n)*' |
31 | 'INFO STOPPING: \'\'\n' |
32 | 'Main loop terminated.\n' |
33 | |
34 | === modified file 'lib/lp/code/scripts/tests/test_reclaim_branch_space.py' |
35 | --- lib/lp/code/scripts/tests/test_reclaim_branch_space.py 2012-01-01 02:58:52 +0000 |
36 | +++ lib/lp/code/scripts/tests/test_reclaim_branch_space.py 2012-03-28 11:45:27 +0000 |
37 | @@ -46,7 +46,8 @@ |
38 | 'cronscripts/reclaimbranchspace.py', []) |
39 | self.assertEqual('', stdout) |
40 | self.assertEqual( |
41 | - 'INFO Creating lockfile: /var/lock/launchpad-reclaimbranchspace.lock\n' |
42 | + 'INFO ' |
43 | + 'Creating lockfile: /var/lock/launchpad-reclaimbranchspace.lock\n' |
44 | 'INFO Reclaimed space for 0 branches.\n', stderr) |
45 | self.assertEqual(0, retcode) |
46 | self.assertTrue( |
47 | @@ -63,16 +64,17 @@ |
48 | retcode, stdout, stderr = run_script( |
49 | 'cronscripts/reclaimbranchspace.py', []) |
50 | self.assertEqual('', stdout) |
51 | - self.assertEqual( |
52 | - 'INFO Creating lockfile: /var/lock/launchpad-reclaimbranchspace.lock\n' |
53 | - 'INFO Running ReclaimBranchSpaceJob (ID %d) in status Waiting\n' |
54 | + self.assertTextMatchesExpressionIgnoreWhitespace( |
55 | + 'INFO ' |
56 | + 'Creating lockfile: /var/lock/launchpad-reclaimbranchspace.lock\n' |
57 | + 'INFO Running <RECLAIM_BRANCH_SPACE branch job \(\d+\) for ' |
58 | + '\d+> \(ID %s\) in status Waiting\n' |
59 | 'INFO Reclaimed space for 1 branches.\n' % reclaim_job.job.id, |
60 | stderr) |
61 | self.assertEqual(0, retcode) |
62 | self.assertFalse( |
63 | os.path.exists(mirrored_path)) |
64 | |
65 | - |
66 | def test_reclaimbranchspace_script_logs_oops(self): |
67 | # If the job fails, an oops is logged. |
68 | db_branch = self.factory.makeAnyBranch() |
69 | |
70 | === modified file 'lib/lp/code/scripts/tests/test_sendbranchmail.py' |
71 | --- lib/lp/code/scripts/tests/test_sendbranchmail.py 2012-01-01 02:58:52 +0000 |
72 | +++ lib/lp/code/scripts/tests/test_sendbranchmail.py 2012-03-28 11:45:27 +0000 |
73 | @@ -50,9 +50,11 @@ |
74 | transaction.commit() |
75 | retcode, stdout, stderr = run_script( |
76 | 'cronscripts/sendbranchmail.py', []) |
77 | - self.assertEqual( |
78 | - 'INFO Creating lockfile: /var/lock/launchpad-sendbranchmail.lock\n' |
79 | - 'INFO Running RevisionMailJob (ID %d) in status Waiting\n' |
80 | + self.assertTextMatchesExpressionIgnoreWhitespace( |
81 | + 'INFO ' |
82 | + 'Creating lockfile: /var/lock/launchpad-sendbranchmail.lock\n' |
83 | + 'INFO Running <REVISION_MAIL branch job \(\d+\) for .*?> ' |
84 | + '\(ID %d\) in status Waiting\n' |
85 | 'INFO Ran 1 RevisionMailJobs.\n' % mail_job.job.id, stderr) |
86 | self.assertEqual('', stdout) |
87 | self.assertEqual(0, retcode) |
88 | @@ -67,7 +69,8 @@ |
89 | retcode, stdout, stderr = run_script( |
90 | 'cronscripts/sendbranchmail.py', []) |
91 | self.assertIn( |
92 | - 'INFO Creating lockfile: /var/lock/launchpad-sendbranchmail.lock\n', |
93 | + 'INFO ' |
94 | + 'Creating lockfile: /var/lock/launchpad-sendbranchmail.lock\n', |
95 | stderr) |
96 | self.assertIn('INFO Job resulted in OOPS:', stderr) |
97 | self.assertIn('INFO Ran 0 RevisionMailJobs.\n', stderr) |
98 | @@ -88,9 +91,11 @@ |
99 | transaction.commit() |
100 | retcode, stdout, stderr = run_script( |
101 | 'cronscripts/sendbranchmail.py', []) |
102 | - self.assertEqual( |
103 | - 'INFO Creating lockfile: /var/lock/launchpad-sendbranchmail.lock\n' |
104 | - 'INFO Running RevisionsAddedJob (ID %d) in status Waiting\n' |
105 | + self.assertTextMatchesExpressionIgnoreWhitespace( |
106 | + 'INFO ' |
107 | + 'Creating lockfile: /var/lock/launchpad-sendbranchmail.lock\n' |
108 | + 'INFO Running <REVISIONS_ADDED_MAIL branch job \(\d+\) ' |
109 | + 'for .*?> \(ID %d\) in status Waiting\n' |
110 | 'INFO Ran 1 RevisionMailJobs.\n' % job.job.id, |
111 | stderr) |
112 | self.assertEqual('', stdout) |
113 | |
114 | === modified file 'lib/lp/registry/tests/test_process_job_sources_cronjob.py' |
115 | --- lib/lp/registry/tests/test_process_job_sources_cronjob.py 2012-01-01 02:58:52 +0000 |
116 | +++ lib/lp/registry/tests/test_process_job_sources_cronjob.py 2012-03-28 11:45:27 +0000 |
117 | @@ -62,7 +62,7 @@ |
118 | returncode, output, error = run_script( |
119 | self.script, ['-v', 'IMembershipNotificationJobSource']) |
120 | self.assertIn( |
121 | - ('DEBUG Running <MembershipNotificationJob ' |
122 | + ('INFO Running <MembershipNotificationJob ' |
123 | 'about ~murdock in ~a-team; status=Waiting>'), |
124 | error) |
125 | self.assertIn('DEBUG MembershipNotificationJob sent email', error) |
126 | @@ -126,7 +126,7 @@ |
127 | returncode, output, error = run_script( |
128 | self.script, ['-v', '--wait', 'MAIN']) |
129 | self.assertTextMatchesExpressionIgnoreWhitespace( |
130 | - ('DEBUG Running <MembershipNotificationJob ' |
131 | + ('INFO Running <MembershipNotificationJob ' |
132 | 'about ~murdock in ~a-team; status=Waiting>'), |
133 | error) |
134 | self.assertIn('DEBUG MembershipNotificationJob sent email', error) |
135 | |
136 | === modified file 'lib/lp/services/job/interfaces/job.py' |
137 | --- lib/lp/services/job/interfaces/job.py 2011-12-24 16:54:44 +0000 |
138 | +++ lib/lp/services/job/interfaces/job.py 2012-03-28 11:45:27 +0000 |
139 | @@ -14,7 +14,6 @@ |
140 | 'ITwistedJobSource', |
141 | 'JobStatus', |
142 | 'LeaseHeld', |
143 | - 'SuspendJobException', |
144 | ] |
145 | |
146 | |
147 | @@ -39,11 +38,6 @@ |
148 | from lp.registry.interfaces.person import IPerson |
149 | |
150 | |
151 | -class SuspendJobException(Exception): |
152 | - """Raised when a running job wants to suspend itself.""" |
153 | - pass |
154 | - |
155 | - |
156 | class LeaseHeld(Exception): |
157 | """Raised when attempting to acquire a list that is already held.""" |
158 | |
159 | @@ -88,6 +82,9 @@ |
160 | class IJob(Interface): |
161 | """Basic attributes of a job.""" |
162 | |
163 | + job_id = Int(title=_( |
164 | + 'A unique identifier for this job.')) |
165 | + |
166 | scheduled_start = Datetime( |
167 | title=_('Time when the IJob was scheduled to start.')) |
168 | |
169 | |
170 | === modified file 'lib/lp/services/job/model/job.py' |
171 | --- lib/lp/services/job/model/job.py 2012-02-24 03:47:44 +0000 |
172 | +++ lib/lp/services/job/model/job.py 2012-03-28 11:45:27 +0000 |
173 | @@ -25,6 +25,7 @@ |
174 | Int, |
175 | Reference, |
176 | ) |
177 | +import transaction |
178 | from zope.interface import implements |
179 | |
180 | from lp.services.database import bulk |
181 | @@ -56,6 +57,10 @@ |
182 | |
183 | implements(IJob) |
184 | |
185 | + @property |
186 | + def job_id(self): |
187 | + return self.id |
188 | + |
189 | scheduled_start = UtcDateTimeCol() |
190 | |
191 | date_created = UtcDateTimeCol() |
192 | @@ -144,31 +149,49 @@ |
193 | expiry = timegm(self.lease_expires.timetuple()) |
194 | return max(0, expiry - time.time()) |
195 | |
196 | - def start(self): |
197 | + def start(self, manage_transaction=False): |
198 | """See `IJob`.""" |
199 | self._set_status(JobStatus.RUNNING) |
200 | self.date_started = datetime.datetime.now(UTC) |
201 | self.date_finished = None |
202 | self.attempt_count += 1 |
203 | + if manage_transaction: |
204 | + transaction.commit() |
205 | |
206 | - def complete(self): |
207 | + def complete(self, manage_transaction=False): |
208 | """See `IJob`.""" |
209 | + # Commit the transaction to update the DB time. |
210 | + if manage_transaction: |
211 | + transaction.commit() |
212 | self._set_status(JobStatus.COMPLETED) |
213 | self.date_finished = datetime.datetime.now(UTC) |
214 | + if manage_transaction: |
215 | + transaction.commit() |
216 | |
217 | - def fail(self): |
218 | + def fail(self, manage_transaction=False): |
219 | """See `IJob`.""" |
220 | + if manage_transaction: |
221 | + transaction.abort() |
222 | self._set_status(JobStatus.FAILED) |
223 | self.date_finished = datetime.datetime.now(UTC) |
224 | + if manage_transaction: |
225 | + transaction.commit() |
226 | |
227 | - def queue(self): |
228 | + def queue(self, manage_transaction=False): |
229 | """See `IJob`.""" |
230 | + # Commit the transaction to update the DB time. |
231 | + if manage_transaction: |
232 | + transaction.commit() |
233 | self._set_status(JobStatus.WAITING) |
234 | self.date_finished = datetime.datetime.now(UTC) |
235 | + if manage_transaction: |
236 | + transaction.commit() |
237 | |
238 | - def suspend(self): |
239 | + def suspend(self, manage_transaction=False): |
240 | """See `IJob`.""" |
241 | self._set_status(JobStatus.SUSPENDED) |
242 | + if manage_transaction: |
243 | + transaction.commit() |
244 | |
245 | def resume(self): |
246 | """See `IJob`.""" |
247 | |
248 | === modified file 'lib/lp/services/job/runner.py' |
249 | --- lib/lp/services/job/runner.py 2012-01-01 02:58:52 +0000 |
250 | +++ lib/lp/services/job/runner.py 2012-03-28 11:45:27 +0000 |
251 | @@ -38,6 +38,7 @@ |
252 | pool, |
253 | ) |
254 | from lazr.delegates import delegates |
255 | +from lazr.jobrunner.jobrunner import JobRunner as LazrJobRunner |
256 | import transaction |
257 | from twisted.internet import reactor |
258 | from twisted.internet.defer import ( |
259 | @@ -61,7 +62,6 @@ |
260 | IJob, |
261 | IRunnableJob, |
262 | LeaseHeld, |
263 | - SuspendJobException, |
264 | ) |
265 | from lp.services.mail.sendmail import ( |
266 | MailController, |
267 | @@ -176,20 +176,24 @@ |
268 | return |
269 | ctrl.send() |
270 | |
271 | - |
272 | -class BaseJobRunner(object): |
273 | + def makeOopsReport(self, oops_config, info): |
274 | + """Generate an OOPS report using the given OOPS configuration.""" |
275 | + return oops_config.create( |
276 | + context=dict(exc_info=info)) |
277 | + |
278 | + |
279 | +class BaseJobRunner(LazrJobRunner): |
280 | """Runner of Jobs.""" |
281 | |
282 | def __init__(self, logger=None, error_utility=None): |
283 | - self.completed_jobs = [] |
284 | - self.incomplete_jobs = [] |
285 | - if logger is None: |
286 | - logger = logging.getLogger() |
287 | - self.logger = logger |
288 | - self.error_utility = error_utility |
289 | self.oops_ids = [] |
290 | - if self.error_utility is None: |
291 | + if error_utility is None: |
292 | self.error_utility = errorlog.globalErrorUtility |
293 | + else: |
294 | + self.error_utility = error_utility |
295 | + super(BaseJobRunner, self).__init__( |
296 | + logger, oops_config=self.error_utility._oops_config, |
297 | + oopsMessage=self.error_utility.oopsMessage) |
298 | |
299 | def acquireLease(self, job): |
300 | self.logger.debug( |
301 | @@ -204,83 +208,8 @@ |
302 | return False |
303 | return True |
304 | |
305 | - @staticmethod |
306 | - def job_str(job): |
307 | - class_name = job.__class__.__name__ |
308 | - ijob_id = removeSecurityProxy(job).job.id |
309 | - return '%s (ID %d)' % (class_name, ijob_id) |
310 | - |
311 | def runJob(self, job): |
312 | - """Attempt to run a job, updating its status as appropriate.""" |
313 | - job = IRunnableJob(job) |
314 | - |
315 | - self.logger.info( |
316 | - 'Running %s in status %s' % ( |
317 | - self.job_str(job), job.status.title)) |
318 | - job.start() |
319 | - transaction.commit() |
320 | - do_retry = False |
321 | - try: |
322 | - try: |
323 | - job.run() |
324 | - except job.retry_error_types, e: |
325 | - if job.attempt_count > job.max_retries: |
326 | - raise |
327 | - self.logger.exception( |
328 | - "Scheduling retry due to %s.", e.__class__.__name__) |
329 | - do_retry = True |
330 | - except SuspendJobException: |
331 | - self.logger.debug("Job suspended itself") |
332 | - job.suspend() |
333 | - self.incomplete_jobs.append(job) |
334 | - except Exception: |
335 | - transaction.abort() |
336 | - job.fail() |
337 | - # Record the failure. |
338 | - transaction.commit() |
339 | - self.incomplete_jobs.append(job) |
340 | - raise |
341 | - else: |
342 | - # Commit transaction to update the DB time. |
343 | - transaction.commit() |
344 | - if do_retry: |
345 | - job.queue() |
346 | - self.incomplete_jobs.append(job) |
347 | - else: |
348 | - job.complete() |
349 | - self.completed_jobs.append(job) |
350 | - # Commit transaction to update job status. |
351 | - transaction.commit() |
352 | - |
353 | - def runJobHandleError(self, job): |
354 | - """Run the specified job, handling errors. |
355 | - |
356 | - Most errors will be logged as Oopses. Jobs in user_error_types won't. |
357 | - The list of complete or incomplete jobs will be updated. |
358 | - """ |
359 | - job = IRunnableJob(job) |
360 | - with self.error_utility.oopsMessage( |
361 | - dict(job.getOopsVars())): |
362 | - try: |
363 | - try: |
364 | - self.logger.debug('Running %r', job) |
365 | - self.runJob(job) |
366 | - except job.user_error_types, e: |
367 | - self.logger.info('Job %r failed with user error %r.' % |
368 | - (job, e)) |
369 | - job.notifyUserError(e) |
370 | - except Exception: |
371 | - info = sys.exc_info() |
372 | - return self._doOops(job, info) |
373 | - except Exception: |
374 | - # This only happens if sending attempting to notify users |
375 | - # about errors fails for some reason (like a misconfigured |
376 | - # email server). |
377 | - self.logger.exception( |
378 | - "Failed to notify users about a failure.") |
379 | - info = sys.exc_info() |
380 | - # Returning the oops says something went wrong. |
381 | - return self.error_utility.raising(info) |
382 | + super(BaseJobRunner, self).runJob(IRunnableJob(job)) |
383 | |
384 | def _doOops(self, job, info): |
385 | """Report an OOPS for the provided job and info. |
386 | @@ -291,6 +220,7 @@ |
387 | """ |
388 | oops = self.error_utility.raising(info) |
389 | job.notifyOops(oops) |
390 | + self._logOopsId(oops['id']) |
391 | return oops |
392 | |
393 | def _logOopsId(self, oops_id): |
394 | @@ -331,9 +261,7 @@ |
395 | continue |
396 | # Commit transaction to clear the row lock. |
397 | transaction.commit() |
398 | - oops = self.runJobHandleError(job) |
399 | - if oops is not None: |
400 | - self._logOopsId(oops['id']) |
401 | + self.runJobHandleError(job) |
402 | |
403 | |
404 | class RunJobCommand(amp.Command): |
405 | |
406 | === modified file 'lib/lp/services/job/tests/test_runner.py' |
407 | --- lib/lp/services/job/tests/test_runner.py 2012-01-01 02:58:52 +0000 |
408 | +++ lib/lp/services/job/tests/test_runner.py 2012-03-28 11:45:27 +0000 |
409 | @@ -8,6 +8,7 @@ |
410 | from textwrap import dedent |
411 | from time import sleep |
412 | |
413 | +from lazr.jobrunner.jobrunner import SuspendJobException |
414 | from testtools.matchers import MatchesRegex |
415 | from testtools.testcase import ExpectedException |
416 | import transaction |
417 | @@ -19,7 +20,6 @@ |
418 | IRunnableJob, |
419 | JobStatus, |
420 | LeaseHeld, |
421 | - SuspendJobException, |
422 | ) |
423 | from lp.services.job.model.job import Job |
424 | from lp.services.job.runner import ( |
425 | @@ -411,8 +411,8 @@ |
426 | self.job = Job() |
427 | |
428 | def __repr__(self): |
429 | - return '<StuckJob(%r, lease_length=%s, delay=%s)>' % ( |
430 | - self.id, self.lease_length, self.delay) |
431 | + return '<%s(%r, lease_length=%s, delay=%s)>' % ( |
432 | + self.__class__.__name__, self.id, self.lease_length, self.delay) |
433 | |
434 | def acquireLease(self): |
435 | return self.job.acquireLease(self.lease_length) |
436 | @@ -550,15 +550,14 @@ |
437 | (1, 1), (len(runner.completed_jobs), len(runner.incomplete_jobs))) |
438 | self.oops_capture.sync() |
439 | oops = self.oopses[0] |
440 | - self.assertEqual( |
441 | - ('TimeoutError', 'Job ran too long.'), |
442 | - (oops['type'], oops['value'])) |
443 | + expected_exception = ('TimeoutError', 'Job ran too long.') |
444 | + self.assertEqual(expected_exception, (oops['type'], oops['value'])) |
445 | self.assertThat(logger.getLogBuffer(), MatchesRegex( |
446 | dedent("""\ |
447 | - INFO Running through Twisted. |
448 | - INFO Running StuckJob \(ID .*\). |
449 | - INFO Running StuckJob \(ID .*\). |
450 | - INFO Job resulted in OOPS: .* |
451 | + INFO Running through Twisted. |
452 | + INFO Running <StuckJob.*?> \(ID .*?\). |
453 | + INFO Running <StuckJob.*?> \(ID .*?\). |
454 | + INFO Job resulted in OOPS: .* |
455 | """))) |
456 | |
457 | def test_timeout_short(self): |
458 | @@ -582,8 +581,8 @@ |
459 | logger.getLogBuffer(), MatchesRegex( |
460 | dedent("""\ |
461 | INFO Running through Twisted. |
462 | - INFO Running ShorterStuckJob \(ID .*\). |
463 | - INFO Running ShorterStuckJob \(ID .*\). |
464 | + INFO Running <ShorterStuckJob.*?> \(ID .*?\). |
465 | + INFO Running <ShorterStuckJob.*?> \(ID .*?\). |
466 | INFO Job resulted in OOPS: %s |
467 | """) % oops['id'])) |
468 | self.assertEqual(('TimeoutError', 'Job ran too long.'), |
469 | |
470 | === modified file 'lib/lp/soyuz/model/packagecopyjob.py' |
471 | --- lib/lp/soyuz/model/packagecopyjob.py 2012-02-24 03:47:44 +0000 |
472 | +++ lib/lp/soyuz/model/packagecopyjob.py 2012-03-28 11:45:27 +0000 |
473 | @@ -11,6 +11,7 @@ |
474 | import logging |
475 | |
476 | from lazr.delegates import delegates |
477 | +from lazr.jobrunner.jobrunner import SuspendJobException |
478 | from storm.locals import ( |
479 | And, |
480 | Int, |
481 | @@ -49,7 +50,6 @@ |
482 | from lp.services.database.stormbase import StormBase |
483 | from lp.services.job.interfaces.job import ( |
484 | JobStatus, |
485 | - SuspendJobException, |
486 | ) |
487 | from lp.services.job.model.job import Job |
488 | from lp.services.job.runner import BaseRunnableJob |
489 | |
490 | === modified file 'lib/lp/soyuz/tests/test_packagecopyjob.py' |
491 | --- lib/lp/soyuz/tests/test_packagecopyjob.py 2012-02-01 11:31:20 +0000 |
492 | +++ lib/lp/soyuz/tests/test_packagecopyjob.py 2012-03-28 11:45:27 +0000 |
493 | @@ -6,6 +6,7 @@ |
494 | import operator |
495 | from textwrap import dedent |
496 | |
497 | +from lazr.jobrunner.jobrunner import SuspendJobException |
498 | from storm.store import Store |
499 | from testtools.content import text_content |
500 | from testtools.matchers import MatchesStructure |
501 | @@ -25,7 +26,6 @@ |
502 | from lp.services.features.testing import FeatureFixture |
503 | from lp.services.job.interfaces.job import ( |
504 | JobStatus, |
505 | - SuspendJobException, |
506 | ) |
507 | from lp.services.webapp.testing import verifyObject |
508 | from lp.soyuz.adapters.overrides import SourceOverride |
509 | |
510 | === modified file 'lib/lp/testing/factory.py' |
511 | --- lib/lp/testing/factory.py 2012-03-26 14:23:39 +0000 |
512 | +++ lib/lp/testing/factory.py 2012-03-28 11:45:27 +0000 |
513 | @@ -48,6 +48,7 @@ |
514 | from bzrlib.merge_directive import MergeDirective2 |
515 | from bzrlib.plugins.builder.recipe import BaseRecipeBranch |
516 | from bzrlib.revision import Revision as BzrRevision |
517 | +from lazr.jobrunner.jobrunner import SuspendJobException |
518 | import pytz |
519 | from pytz import UTC |
520 | import simplejson |
521 | @@ -233,7 +234,6 @@ |
522 | IEmailAddressSet, |
523 | ) |
524 | from lp.services.identity.model.account import Account |
525 | -from lp.services.job.interfaces.job import SuspendJobException |
526 | from lp.services.librarian.interfaces import ILibraryFileAliasSet |
527 | from lp.services.mail.signedmessage import SignedMessage |
528 | from lp.services.messages.model.message import ( |
529 | |
530 | === modified file 'lib/lp/translations/scripts/tests/test_packaging_translations.py' |
531 | --- lib/lp/translations/scripts/tests/test_packaging_translations.py 2012-01-01 02:58:52 +0000 |
532 | +++ lib/lp/translations/scripts/tests/test_packaging_translations.py 2012-03-28 11:45:27 +0000 |
533 | @@ -33,10 +33,10 @@ |
534 | matcher = MatchesRegex(dedent("""\ |
535 | INFO Creating lockfile: /var/lock/launchpad-jobcronscript.lock |
536 | INFO Running synchronously. |
537 | - INFO Running TranslationMergeJob \(ID .*\) in status Waiting |
538 | + INFO Running <.*?TranslationMergeJob.*?> \(ID .*\) in status Waiting |
539 | INFO Merging .* and .* in Ubuntu Distroseries.* |
540 | INFO Deleted POTMsgSets: 1. TranslationMessages: 1. |
541 | - INFO Running TranslationSplitJob \(ID .*\) in status Waiting |
542 | + INFO Running <.*?TranslationSplitJob.*?> \(ID .*\) in status Waiting |
543 | INFO Splitting .* and .* in Ubuntu Distroseries.* |
544 | INFO 1 entries split. |
545 | INFO Ran 1 TranslationMergeJob jobs. |
546 | |
547 | === modified file 'setup.py' |
548 | --- setup.py 2012-03-09 04:33:37 +0000 |
549 | +++ setup.py 2012-03-28 11:45:27 +0000 |
550 | @@ -47,6 +47,7 @@ |
551 | 'lazr.enum', |
552 | 'lazr.lifecycle', |
553 | 'lazr.restful', |
554 | + 'lazr.jobrunner', |
555 | 'lazr.smtptest', |
556 | 'lazr.testing', |
557 | 'lazr.uri', |
558 | |
559 | === modified file 'versions.cfg' |
560 | --- versions.cfg 2012-03-26 22:45:24 +0000 |
561 | +++ versions.cfg 2012-03-28 11:45:27 +0000 |
562 | @@ -5,11 +5,13 @@ |
563 | # Alphabetical, case-insensitive, please! :-) |
564 | |
565 | ampoule = 0.2.0 |
566 | -amqplib = 0.6.1 |
567 | +amqplib = 1.0.2 |
568 | +anyjson = 0.3.1 |
569 | argparse = 1.2.1 |
570 | BeautifulSoup = 3.1.0.1 |
571 | bson = 0.3.2 |
572 | bzr = 2.5.0dev2-r6152 |
573 | +celery = 2.5.1 |
574 | Chameleon = 2.4.0 |
575 | ClientForm = 0.2.10 |
576 | cssutils = 0.9.7 |
577 | @@ -26,11 +28,13 @@ |
578 | grokcore.component = 1.6 |
579 | html5browser = 0.0.9 |
580 | httplib2 = 0.6.0 |
581 | +importlib = 1.0.2 |
582 | ipython = 0.9.1 |
583 | iso8601 = 0.1.4 |
584 | jsautobuild = 0.2 |
585 | Jinja2 = 2.2 |
586 | keyring = 0.6.2 |
587 | +kombu = 2.1.1 |
588 | launchpadlib = 1.9.12 |
589 | lazr.amqp = 0.1 |
590 | lazr.authentication = 0.1.1 |
591 | @@ -38,6 +42,7 @@ |
592 | lazr.config = 1.1.3 |
593 | lazr.delegates = 1.2.0 |
594 | lazr.enum = 1.1.3 |
595 | +lazr.jobrunner = 0.2 |
596 | lazr.lifecycle = 1.1 |
597 | lazr.restful = 0.19.6 |
598 | lazr.restfulclient = 0.12.0 |
599 | @@ -78,6 +83,7 @@ |
600 | pymongo = 2.1.1 |
601 | pyOpenSSL = 0.10 |
602 | pystache = 0.3.1 |
603 | +python-dateutil = 1.5 |
604 | python-memcached = 1.48 |
605 | # 2.2.1 with the one-liner Expect: 100-continue fix from |
606 | # lp:~wgrant/python-openid/python-openid-2.2.1-fix676372. |
Not landable yet because it assumes lazr.jobrunner is a development egg. "../lazr.jobrunner" should not be listed in buildout.cfg, and a version of lazr.jobrunner should be specified in versions.cfg.
runJob may not need to be overridden in BaseJobRunner. If not, please remove it.
job_str may not need to be overridden in BaseJobRunner. If not, please remove it.