Launchpad itself

Merge lp:~abentley/launchpad/twistedjob-enhancements into lp:launchpad

  1. twistedjob-enhancements
  2. Merge into devel
Proposed by Aaron Bentley
Status: Merged
Approved by: Aaron Bentley
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~abentley/launchpad/twistedjob-enhancements
Merge into: lp:launchpad
Diff against target: 254 lines (+117/-6)
8 files modified
lib/lp/code/interfaces/branchmergeproposal.py (+7/-0)
lib/lp/code/model/branchmergeproposaljob.py (+16/-0)
lib/lp/code/model/tests/test_branchmergeproposals.py (+32/-3)
lib/lp/code/scripts/tests/test_update_preview_diffs.py (+28/-3)
lib/lp/services/job/interfaces/job.py (+3/-0)
lib/lp/services/job/model/job.py (+10/-0)
lib/lp/services/job/runner.py (+3/-0)
lib/lp/services/job/tests/test_job.py (+18/-0)
To merge this branch: bzr merge lp:~abentley/launchpad/twistedjob-enhancements
Reviewer Review Type Date Requested Status
Paul Hummer (community) Approve
Review via email: mp+15977@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Aaron Bentley (abentley) wrote :

= Summary =
Provide misc functionality for converting jobs to use Twisted.

== Proposed fix ==
Add IBranchMergeProposalJob.id and BranchMergeProposalJob.get, which when
invoked in subclasses returns the BranchMergeProposalJob with that id as an
instance of that subclass.

Add an additional message to JobRunner indicating how many jobs did
not complete.

== Pre-implementation notes ==
None of this had a pre-implementation call, though it's basically all required
for the followon branches.

== Implementation details ==
Err...

== Tests ==
bin/test -t test_job -t test_update_preview_diffs -t test_branchmergeproposals

== Demo and Q/A ==
Look at the logs produced by update_preview_diffs on
/x/launchpad.net-logs/scripts/asuka/bzrsyncd. They should have a line that
says "x IUpdatePreviewDiffSource jobs did not complete.".

Revision history for this message
Paul Hummer (rockstar) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/code/interfaces/branchmergeproposal.py'
--- lib/lp/code/interfaces/branchmergeproposal.py 2009-12-07 03:56:18 +0000
+++ lib/lp/code/interfaces/branchmergeproposal.py 2009-12-18 16:00:38 +0000
@@ -474,6 +474,10 @@
474class IBranchMergeProposalJob(Interface):474class IBranchMergeProposalJob(Interface):
475 """A Job related to a Branch Merge Proposal."""475 """A Job related to a Branch Merge Proposal."""
476476
477 id = Int(
478 title=_('DB ID'), required=True, readonly=True,
479 description=_("The tracking number for this job."))
480
477 branch_merge_proposal = Object(481 branch_merge_proposal = Object(
478 title=_('The BranchMergeProposal this job is about'),482 title=_('The BranchMergeProposal this job is about'),
479 schema=IBranchMergeProposal, required=True)483 schema=IBranchMergeProposal, required=True)
@@ -587,6 +591,9 @@
587 def create(bmp):591 def create(bmp):
588 """Create a job to update the diff for this merge proposal."""592 """Create a job to update the diff for this merge proposal."""
589593
594 def get(id):
595 """Return the UpdatePreviewDiffJob with this id."""
596
590 def iterReady():597 def iterReady():
591 """Iterate through jobs ready to update preview diffs."""598 """Iterate through jobs ready to update preview diffs."""
592599
593600
=== modified file 'lib/lp/code/model/branchmergeproposaljob.py'
--- lib/lp/code/model/branchmergeproposaljob.py 2009-11-03 21:05:14 +0000
+++ lib/lp/code/model/branchmergeproposaljob.py 2009-12-18 16:00:38 +0000
@@ -164,6 +164,22 @@
164 return cls(job)164 return cls(job)
165165
166 @classmethod166 @classmethod
167 def get(cls, job_id):
168 """Get a job by id.
169
170 :return: the BranchMergeProposalJob with the specified id, as the
171 current BranchMergeProposalJobDereived subclass.
172 :raises: SQLObjectNotFound if there is no job with the specified id,
173 or its job_type does not match the desired subclass.
174 """
175 job = BranchMergeProposalJob.get(job_id)
176 if job.job_type != cls.class_job_type:
177 raise SQLObjectNotFound(
178 'No object found with id %d and type %s' % (job_id,
179 cls.class_job_type.title))
180 return cls(job)
181
182 @classmethod
167 def iterReady(klass):183 def iterReady(klass):
168 """Iterate through all ready BranchMergeProposalJobs."""184 """Iterate through all ready BranchMergeProposalJobs."""
169 from lp.code.model.branch import Branch185 from lp.code.model.branch import Branch
170186
=== modified file 'lib/lp/code/model/tests/test_branchmergeproposals.py'
--- lib/lp/code/model/tests/test_branchmergeproposals.py 2009-12-08 09:02:43 +0000
+++ lib/lp/code/model/tests/test_branchmergeproposals.py 2009-12-18 16:00:38 +0000
@@ -41,10 +41,11 @@
41 BRANCH_MERGE_PROPOSAL_FINAL_STATES as FINAL_STATES,41 BRANCH_MERGE_PROPOSAL_FINAL_STATES as FINAL_STATES,
42 IBranchMergeProposal, IBranchMergeProposalGetter, IBranchMergeProposalJob,42 IBranchMergeProposal, IBranchMergeProposalGetter, IBranchMergeProposalJob,
43 ICreateMergeProposalJob, ICreateMergeProposalJobSource,43 ICreateMergeProposalJob, ICreateMergeProposalJobSource,
44 IMergeProposalCreatedJob, notify_modified)44 IMergeProposalCreatedJob, IUpdatePreviewDiffJobSource, notify_modified)
45from lp.code.model.branchmergeproposaljob import (45from lp.code.model.branchmergeproposaljob import (
46 BranchMergeProposalJob, BranchMergeProposalJobType,46 BranchMergeProposalJob, BranchMergeProposalJobDerived,
47 CreateMergeProposalJob, MergeProposalCreatedJob, UpdatePreviewDiffJob)47 BranchMergeProposalJobType, CreateMergeProposalJob,
48 MergeProposalCreatedJob, UpdatePreviewDiffJob)
48from lp.code.model.branchmergeproposal import (49from lp.code.model.branchmergeproposal import (
49 BranchMergeProposal, BranchMergeProposalGetter, is_valid_transition)50 BranchMergeProposal, BranchMergeProposalGetter, is_valid_transition)
50from lp.code.model.diff import StaticDiff51from lp.code.model.diff import StaticDiff
@@ -1235,6 +1236,30 @@
1235 verifyObject(IBranchMergeProposalJob, job)1236 verifyObject(IBranchMergeProposalJob, job)
12361237
12371238
1239class TestBranchMergeProposalJobDerived(TestCaseWithFactory):
1240 """Test the behaviour of the BranchMergeProposalJobDerived base class."""
1241
1242 layer = LaunchpadZopelessLayer
1243
1244 def test_get(self):
1245 """Ensure get returns or raises appropriately.
1246
1247 It's an error to call get on BranchMergeProposalJobDerived-- it must
1248 be called on a subclass. An object is returned only if the job id
1249 and job type match the request. If no suitable object can be found,
1250 SQLObjectNotFound is raised.
1251 """
1252 bmp = self.factory.makeBranchMergeProposal()
1253 job = MergeProposalCreatedJob.create(bmp)
1254 transaction.commit()
1255 self.assertRaises(
1256 AttributeError, BranchMergeProposalJobDerived.get, job.id)
1257 self.assertRaises(SQLObjectNotFound, UpdatePreviewDiffJob.get, job.id)
1258 self.assertRaises(
1259 SQLObjectNotFound, MergeProposalCreatedJob.get, job.id + 1)
1260 self.assertEqual(job, MergeProposalCreatedJob.get(job.id))
1261
1262
1238class TestMergeProposalCreatedJob(TestCaseWithFactory):1263class TestMergeProposalCreatedJob(TestCaseWithFactory):
12391264
1240 layer = LaunchpadZopelessLayer1265 layer = LaunchpadZopelessLayer
@@ -1849,6 +1874,10 @@
18491874
1850 layer = LaunchpadZopelessLayer1875 layer = LaunchpadZopelessLayer
18511876
1877 def test_implement_interface(self):
1878 """UpdatePreviewDiffJob implements IUpdatePreviewDiffJobSource."""
1879 verifyObject(IUpdatePreviewDiffJobSource, UpdatePreviewDiffJob)
1880
1852 def test_run(self):1881 def test_run(self):
1853 self.useBzrBranches()1882 self.useBzrBranches()
1854 bmp = self.createExampleMerge()[0]1883 bmp = self.createExampleMerge()[0]
18551884
=== modified file 'lib/lp/code/scripts/tests/test_update_preview_diffs.py'
--- lib/lp/code/scripts/tests/test_update_preview_diffs.py 2009-10-17 14:06:03 +0000
+++ lib/lp/code/scripts/tests/test_update_preview_diffs.py 2009-12-18 16:00:38 +0000
@@ -11,6 +11,7 @@
11from canonical.testing import ZopelessAppServerLayer11from canonical.testing import ZopelessAppServerLayer
12from lp.testing import TestCaseWithFactory12from lp.testing import TestCaseWithFactory
13from canonical.launchpad.scripts.tests import run_script13from canonical.launchpad.scripts.tests import run_script
14from canonical.launchpad.webapp import errorlog
14from lp.code.model.branchmergeproposal import BranchMergeProposal15from lp.code.model.branchmergeproposal import BranchMergeProposal
15from lp.code.model.branchmergeproposaljob import UpdatePreviewDiffJob16from lp.code.model.branchmergeproposaljob import UpdatePreviewDiffJob
1617
@@ -19,8 +20,7 @@
1920
20 layer = ZopelessAppServerLayer21 layer = ZopelessAppServerLayer
2122
22 def test_update_preview_diffs(self):23 def create_preview_diff_job(self):
23 """Ensure update_preview_diffs runs and generates diffs."""
24 self.useTempBzrHome()24 self.useTempBzrHome()
25 target, target_tree = self.createMirroredBranchAndTree()25 target, target_tree = self.createMirroredBranchAndTree()
26 target_tree.bzrdir.root_transport.put_bytes('foo', 'foo\n')26 target_tree.bzrdir.root_transport.put_bytes('foo', 'foo\n')
@@ -38,11 +38,36 @@
38 job = UpdatePreviewDiffJob.create(bmp)38 job = UpdatePreviewDiffJob.create(bmp)
39 self.assertIs(None, bmp.preview_diff)39 self.assertIs(None, bmp.preview_diff)
40 transaction.commit()40 transaction.commit()
41 return job, bmp, source_tree
42
43 def test_update_preview_diffs(self):
44 """Ensure update_preview_diffs runs and generates diffs."""
45 job, bmp, source_tree = self.create_preview_diff_job()
41 retcode, stdout, stderr = run_script(46 retcode, stdout, stderr = run_script(
42 'cronscripts/update_preview_diffs.py', [])47 'cronscripts/update_preview_diffs.py', [])
43 self.assertEqual(0, retcode)48 self.assertEqual(0, retcode)
44 self.assertEqual('', stdout)49 self.assertEqual('', stdout)
45 self.assertEqual(50 self.assertEqual(
46 'INFO creating lockfile\n'51 'INFO creating lockfile\n'
47 'INFO Ran 1 IUpdatePreviewDiffJobSource jobs.\n', stderr)52 'INFO Ran 1 IUpdatePreviewDiffJobSource jobs.\n'
53 'INFO 0 IUpdatePreviewDiffJobSource jobs did not complete.\n',
54 stderr)
48 self.assertIsNot(None, bmp.preview_diff)55 self.assertIsNot(None, bmp.preview_diff)
56
57 def test_update_preview_diffs_oops(self):
58 """Ensure update_preview_diffs runs and generates diffs."""
59 job, bmp, source_tree = self.create_preview_diff_job()
60 source_tree.bzrdir.root_transport.delete_tree('.bzr')
61 error_utility = errorlog.ErrorReportingUtility()
62 error_utility.configure('update_preview_diffs')
63 old_id = error_utility.getLastOopsReport().id
64 retcode, stdout, stderr = run_script(
65 'cronscripts/update_preview_diffs.py', [])
66 self.assertEqual(0, retcode)
67 self.assertIn(
68 'INFO 1 IUpdatePreviewDiffJobSource jobs did not complete.\n',
69 stderr)
70 self.assertIn('INFO Job resulted in OOPS:', stderr)
71 new_id = error_utility.getLastOopsReport().id
72 self.assertNotEqual(old_id, new_id)
73
4974
=== modified file 'lib/lp/services/job/interfaces/job.py'
--- lib/lp/services/job/interfaces/job.py 2009-09-02 21:09:48 +0000
+++ lib/lp/services/job/interfaces/job.py 2009-12-18 16:00:38 +0000
@@ -82,6 +82,9 @@
82 def acquireLease(duration=300):82 def acquireLease(duration=300):
83 """Acquire the lease for this Job, or raise LeaseHeld."""83 """Acquire the lease for this Job, or raise LeaseHeld."""
8484
85 def getTimeout():
86 """Determine how long this job can run before timing out."""
87
85 def start():88 def start():
86 """Mark the job as started."""89 """Mark the job as started."""
8790
8891
=== modified file 'lib/lp/services/job/model/job.py'
--- lib/lp/services/job/model/job.py 2009-07-17 00:26:05 +0000
+++ lib/lp/services/job/model/job.py 2009-12-18 16:00:38 +0000
@@ -7,6 +7,7 @@
7__all__ = ['InvalidTransition', 'Job', 'JobStatus']7__all__ = ['InvalidTransition', 'Job', 'JobStatus']
88
99
10from calendar import timegm
10import datetime11import datetime
11import time12import time
1213
@@ -84,6 +85,15 @@
84 UTC)85 UTC)
85 self.lease_expires = expiry86 self.lease_expires = expiry
8687
88 def getTimeout(self):
89 """Return the number of seconds until the job should time out.
90
91 Jobs timeout when their leases expire. If the lease for this job has
92 already expired, return 0.
93 """
94 expiry = timegm(self.lease_expires.timetuple())
95 return max(0, expiry - time.time())
96
87 def start(self):97 def start(self):
88 """See `IJob`."""98 """See `IJob`."""
89 self._set_status(JobStatus.RUNNING)99 self._set_status(JobStatus.RUNNING)
90100
=== modified file 'lib/lp/services/job/runner.py'
--- lib/lp/services/job/runner.py 2009-11-26 16:06:55 +0000
+++ lib/lp/services/job/runner.py 2009-12-18 16:00:38 +0000
@@ -176,3 +176,6 @@
176 self.logger.info(176 self.logger.info(
177 'Ran %d %s jobs.',177 'Ran %d %s jobs.',
178 len(runner.completed_jobs), self.source_interface.__name__)178 len(runner.completed_jobs), self.source_interface.__name__)
179 self.logger.info(
180 '%d %s jobs did not complete.',
181 len(runner.incomplete_jobs), self.source_interface.__name__)
179182
=== modified file 'lib/lp/services/job/tests/test_job.py'
--- lib/lp/services/job/tests/test_job.py 2009-11-13 06:59:24 +0000
+++ lib/lp/services/job/tests/test_job.py 2009-12-18 16:00:38 +0000
@@ -225,6 +225,24 @@
225 job.acquireLease(-1)225 job.acquireLease(-1)
226 job.acquireLease()226 job.acquireLease()
227227
228 def test_acquireLeaseTimeout(self):
229 """Test that getTimeout correctly calculates value from lease.
230
231 The imprecision is because leases are relative to the current time,
232 and the current time may have changed by the time we get to
233 job.getTimeout() <= 300.
234 """
235 job = Job()
236 job.acquireLease(300)
237 self.assertTrue(job.getTimeout() > 0)
238 self.assertTrue(job.getTimeout() <= 300)
239
240 def test_acquireLeaseTimeoutExpired(self):
241 """Expired leases don't produce negative timeouts."""
242 job = Job()
243 job.acquireLease(-300)
244 self.assertEqual(0, job.getTimeout())
245
228246
229def test_suite():247def test_suite():
230 return TestLoader().loadTestsFromName(__name__)248 return TestLoader().loadTestsFromName(__name__)