Merge lp:~julian-edwards/launchpad/suspend-running-jobs-bug-788612 into lp:launchpad

Proposed by Julian Edwards
Status: Merged
Approved by: Aaron Bentley
Approved revision: no longer in the source branch.
Merged at revision: 13128
Proposed branch: lp:~julian-edwards/launchpad/suspend-running-jobs-bug-788612
Merge into: lp:launchpad
Diff against target: 113 lines (+28/-2)
5 files modified
lib/lp/services/job/interfaces/job.py (+6/-0)
lib/lp/services/job/model/job.py (+1/-0)
lib/lp/services/job/runner.py (+5/-0)
lib/lp/services/job/tests/test_job.py (+3/-2)
lib/lp/services/job/tests/test_runner.py (+13/-0)
To merge this branch: bzr merge lp:~julian-edwards/launchpad/suspend-running-jobs-bug-788612
Reviewer Review Type Date Requested Status
Aaron Bentley (community) Approve
Review via email: mp+62494@code.launchpad.net

Commit message

[r=abentley][bug=788612] Allow running Jobs to suspend themselves

Description of the change

= Summary =
Allow running Jobs to suspend themselves.

== Pre-implementation notes ==
Talked to Aaron and we agreed this strategy.

== Implementation details ==
There's a new exception SuspendJobError defined in interfaces/job.py which any job can raise. It is caught in the main runner code which will subsequently change the job's status to SUSPENDED.

== Tests ==
bin/test -cvv test_runner test_runJob_with_SuspendJobError

== Demo and Q/A ==
n/a

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/services/job/tests/test_runner.py
  lib/lp/services/job/runner.py
  lib/lp/services/job/interfaces/job.py
  lib/lp/services/job/model/job.py

To post a comment you must log in.
Revision history for this message
Aaron Bentley (abentley) wrote :

I think SuspendJobError is one of those exceptions that's not actually an error, but no need to bikeshed on the name.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/services/job/interfaces/job.py'
2--- lib/lp/services/job/interfaces/job.py 2011-05-19 15:15:16 +0000
3+++ lib/lp/services/job/interfaces/job.py 2011-05-27 08:19:28 +0000
4@@ -14,6 +14,7 @@
5 'ITwistedJobSource',
6 'JobStatus',
7 'LeaseHeld',
8+ 'SuspendJobException',
9 ]
10
11
12@@ -35,6 +36,11 @@
13 from canonical.launchpad import _
14
15
16+class SuspendJobException(Exception):
17+ """Raised when a running job wants to suspend itself."""
18+ pass
19+
20+
21 class LeaseHeld(Exception):
22 """Raised when attempting to acquire a list that is already held."""
23
24
25=== modified file 'lib/lp/services/job/model/job.py'
26--- lib/lp/services/job/model/job.py 2011-05-19 15:15:16 +0000
27+++ lib/lp/services/job/model/job.py 2011-05-27 08:19:28 +0000
28@@ -78,6 +78,7 @@
29 JobStatus.RUNNING:
30 (JobStatus.COMPLETED,
31 JobStatus.FAILED,
32+ JobStatus.SUSPENDED,
33 JobStatus.WAITING),
34 JobStatus.FAILED: (),
35 JobStatus.COMPLETED: (),
36
37=== modified file 'lib/lp/services/job/runner.py'
38--- lib/lp/services/job/runner.py 2011-05-19 15:15:16 +0000
39+++ lib/lp/services/job/runner.py 2011-05-27 08:19:28 +0000
40@@ -51,6 +51,7 @@
41 IJob,
42 IRunnableJob,
43 LeaseHeld,
44+ SuspendJobException,
45 )
46 from lp.services.mail.sendmail import MailController
47 from lp.services.scripts.base import LaunchpadCronScript
48@@ -182,6 +183,10 @@
49 self.logger.exception(
50 "Scheduling retry due to %s.", e.__class__.__name__)
51 do_retry = True
52+ except SuspendJobException:
53+ self.logger.debug("Job suspended itself")
54+ job.suspend()
55+ self.incomplete_jobs.append(job)
56 except Exception:
57 self.logger.exception("Job execution raised an exception.")
58 transaction.abort()
59
60=== modified file 'lib/lp/services/job/tests/test_job.py'
61--- lib/lp/services/job/tests/test_job.py 2011-03-23 15:30:02 +0000
62+++ lib/lp/services/job/tests/test_job.py 2011-05-27 08:19:28 +0000
63@@ -169,9 +169,10 @@
64 JobStatus.SUSPENDED)
65
66 def test_suspend_when_running(self):
67- """When a job is running, attempting to suspend is invalid."""
68+ """When a job is running, attempting to suspend is valid."""
69 job = Job(_status=JobStatus.RUNNING)
70- self.assertRaises(InvalidTransition, job.suspend)
71+ job.suspend()
72+ self.assertEqual(JobStatus.SUSPENDED, job.status)
73
74 def test_suspend_when_completed(self):
75 """When a job is completed, attempting to suspend is invalid."""
76
77=== modified file 'lib/lp/services/job/tests/test_runner.py'
78--- lib/lp/services/job/tests/test_runner.py 2011-05-25 14:07:19 +0000
79+++ lib/lp/services/job/tests/test_runner.py 2011-05-27 08:19:28 +0000
80@@ -22,6 +22,7 @@
81 from lp.services.job.interfaces.job import (
82 IRunnableJob,
83 JobStatus,
84+ SuspendJobException,
85 )
86 from lp.services.job.model.job import Job
87 from lp.services.job.runner import (
88@@ -38,6 +39,7 @@
89 TestCaseWithFactory,
90 ZopeTestInSubProcess,
91 )
92+from lp.testing.fakemethod import FakeMethod
93 from lp.testing.mail_helpers import pop_notifications
94
95
96@@ -363,6 +365,17 @@
97 runner.runJobHandleError(job)
98 self.assertEqual(1, len(self.oopses))
99
100+ def test_runJob_with_SuspendJobException(self):
101+ # A job that raises SuspendJobError should end up suspended.
102+ job = NullJob('suspended')
103+ job.run = FakeMethod(failure=SuspendJobException())
104+ runner = JobRunner([job])
105+ runner.runJob(job)
106+
107+ self.assertEqual(JobStatus.SUSPENDED, job.status)
108+ self.assertNotIn(job, runner.completed_jobs)
109+ self.assertIn(job, runner.incomplete_jobs)
110+
111
112 class StuckJob(BaseRunnableJob):
113 """Simulation of a job that stalls."""