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
=== modified file 'lib/lp/services/job/runner.py'
--- lib/lp/services/job/runner.py 2009-09-04 18:53:12 +0000
+++ lib/lp/services/job/runner.py 2009-09-24 05:51:14 +0000
@@ -122,6 +122,8 @@
122 except Exception:122 except Exception:
123 transaction.abort()123 transaction.abort()
124 job.fail()124 job.fail()
125 # Record the failure.
126 transaction.commit()
125 self.incomplete_jobs.append(job)127 self.incomplete_jobs.append(job)
126 raise128 raise
127 else:129 else:
128130
=== modified file 'lib/lp/services/job/tests/test_runner.py'
--- lib/lp/services/job/tests/test_runner.py 2009-09-02 21:09:48 +0000
+++ lib/lp/services/job/tests/test_runner.py 2009-09-24 05:51:14 +0000
@@ -52,6 +52,17 @@
52 return 'appending a string to a list'52 return 'appending a string to a list'
5353
5454
55class RaisingJobException(Exception):
56 """Raised by the RaisingJob when run."""
57
58
59class RaisingJob(NullJob):
60 """A job that raises when it runs."""
61
62 def run(self):
63 raise RaisingJobException(self.message)
64
65
55class TestJobRunner(TestCaseWithFactory):66class TestJobRunner(TestCaseWithFactory):
56 """Ensure JobRunner behaves as expected."""67 """Ensure JobRunner behaves as expected."""
5768
@@ -202,6 +213,16 @@
202 runner = JobRunner([Runnable()])213 runner = JobRunner([Runnable()])
203 self.assertRaises(AttributeError, runner.runAll)214 self.assertRaises(AttributeError, runner.runAll)
204215
216 def test_runJob_records_failure(self):
217 """When a job fails, the failure needs to be recorded."""
218 job = RaisingJob('boom')
219 runner = JobRunner([job])
220 self.assertRaises(RaisingJobException, runner.runJob, job)
221 # Abort the transaction to confirm that the update of the job status
222 # has been committed.
223 transaction.abort()
224 self.assertEqual(JobStatus.FAILED, job.job.status)
225
205226
206def test_suite():227def test_suite():
207 return TestLoader().loadTestsFromName(__name__)228 return TestLoader().loadTestsFromName(__name__)

Subscribers

People subscribed via source and target branches

to status/vote changes: