Launchpad itself

Merge lp:~thumper/launchpad/job-fail into lp:launchpad/db-devel

  1. job-fail
  2. Merge into db-devel
Proposed by Tim Penhey
Status: Merged
Approved by: Brad Crittenden
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~thumper/launchpad/job-fail
Merge into: lp:launchpad/db-devel
Diff against target: 51 lines
2 files modified
lib/lp/services/job/runner.py (+2/-0)
lib/lp/services/job/tests/test_runner.py (+21/-0)
To merge this branch: bzr merge lp:~thumper/launchpad/job-fail
Reviewer Review Type Date Requested Status
Brad Crittenden (community) release-critical Approve
Michael Hudson-Doyle Approve
Review via email: mp+12328@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Tim Penhey (thumper) wrote :

The job runner fails to record a failure when the last job fails, as it doesn't commit the recording of the failure. This is the source of all the non-running running jobs in the db.

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

It all looks fine apart from this comment in the test:

# Abort the transaction to confirm that the job status has been
# updated.

I suggest ".. to confirm that the update of the job status has been committed."

review: Approve
Revision history for this message
Tim Penhey (thumper) wrote :

On Thu, 24 Sep 2009 17:45:15 Michael Hudson wrote:
> Review: Approve
> It all looks fine apart from this comment in the test:
>
> # Abort the transaction to confirm that the job status has been
> # updated.
>
> I suggest ".. to confirm that the update of the job status has been
> committed."

Thanks. Updated.

Revision history for this message
Brad Crittenden (bac) :
review: Approve (release-critical)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/services/job/runner.py'
2--- lib/lp/services/job/runner.py 2009-09-04 18:53:12 +0000
3+++ lib/lp/services/job/runner.py 2009-09-24 05:51:14 +0000
4@@ -122,6 +122,8 @@
5 except Exception:
6 transaction.abort()
7 job.fail()
8+ # Record the failure.
9+ transaction.commit()
10 self.incomplete_jobs.append(job)
11 raise
12 else:
13
14=== modified file 'lib/lp/services/job/tests/test_runner.py'
15--- lib/lp/services/job/tests/test_runner.py 2009-09-02 21:09:48 +0000
16+++ lib/lp/services/job/tests/test_runner.py 2009-09-24 05:51:14 +0000
17@@ -52,6 +52,17 @@
18 return 'appending a string to a list'
19
20
21+class RaisingJobException(Exception):
22+ """Raised by the RaisingJob when run."""
23+
24+
25+class RaisingJob(NullJob):
26+ """A job that raises when it runs."""
27+
28+ def run(self):
29+ raise RaisingJobException(self.message)
30+
31+
32 class TestJobRunner(TestCaseWithFactory):
33 """Ensure JobRunner behaves as expected."""
34
35@@ -202,6 +213,16 @@
36 runner = JobRunner([Runnable()])
37 self.assertRaises(AttributeError, runner.runAll)
38
39+ def test_runJob_records_failure(self):
40+ """When a job fails, the failure needs to be recorded."""
41+ job = RaisingJob('boom')
42+ runner = JobRunner([job])
43+ self.assertRaises(RaisingJobException, runner.runJob, job)
44+ # Abort the transaction to confirm that the update of the job status
45+ # has been committed.
46+ transaction.abort()
47+ self.assertEqual(JobStatus.FAILED, job.job.status)
48+
49
50 def test_suite():
51 return TestLoader().loadTestsFromName(__name__)

Subscribers

People subscribed via source and target branches

to status/vote changes: