Merge ~cjwatson/launchpad:job-retry-no-oops into launchpad:master

Proposed by Colin Watson
Status: Merged
Approved by: Colin Watson
Approved revision: 3c1cbd915f7fbb3a4ba17df9561aa456b6557ac4
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~cjwatson/launchpad:job-retry-no-oops
Merge into: launchpad:master
Diff against target: 67 lines (+18/-4)
2 files modified
constraints.txt (+1/-1)
lib/lp/services/job/tests/test_runner.py (+17/-3)
Reviewer Review Type Date Requested Status
Tom Wardill (community) Approve
Review via email: mp+373804@code.launchpad.net

Commit message

Upgrade to lazr.jobrunner 0.16, and test for lack of OOPSes on retry

Description of the change

lazr.jobrunner 0.16 also adds Python 3 support, although we'll need to upgrade to Celery 4.3 or newer before being able to take advantage of that.

The retry OOPS fix comes from https://code.launchpad.net/~cjwatson/lazr.jobrunner/quieter-retry-handling/+merge/369697 (merged).

This is essentially the same as https://code.launchpad.net/~cjwatson/launchpad/job-retry-no-oops/+merge/372556, converted to git and rebased on master.

To post a comment you must log in.
Revision history for this message
Tom Wardill (twom) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/constraints.txt b/constraints.txt
index fa23f05..df3cea6 100644
--- a/constraints.txt
+++ b/constraints.txt
@@ -286,7 +286,7 @@ lazr.batchnavigator==1.2.11
286lazr.config==2.2.1286lazr.config==2.2.1
287lazr.delegates==2.0.4287lazr.delegates==2.0.4
288lazr.enum==1.1.3288lazr.enum==1.1.3
289lazr.jobrunner==0.14289lazr.jobrunner==0.16
290lazr.lifecycle==1.1290lazr.lifecycle==1.1
291lazr.restful==0.20.1291lazr.restful==0.20.1
292lazr.restfulclient==0.13.2292lazr.restfulclient==0.13.2
diff --git a/lib/lp/services/job/tests/test_runner.py b/lib/lp/services/job/tests/test_runner.py
index d1001ff..62a2376 100644
--- a/lib/lp/services/job/tests/test_runner.py
+++ b/lib/lp/services/job/tests/test_runner.py
@@ -44,6 +44,7 @@ from lp.services.job.runner import (
44 TwistedJobRunner,44 TwistedJobRunner,
45 )45 )
46from lp.services.log.logger import BufferLogger46from lp.services.log.logger import BufferLogger
47from lp.services.scripts.logger import OopsHandler
47from lp.services.timeline.requesttimeline import get_request_timeline48from lp.services.timeline.requesttimeline import get_request_timeline
48from lp.services.timeout import (49from lp.services.timeout import (
49 get_default_timeout_function,50 get_default_timeout_function,
@@ -366,18 +367,29 @@ class TestJobRunner(TestCaseWithFactory):
366367
367 def test_runJobHandleErrors_user_error_no_oops(self):368 def test_runJobHandleErrors_user_error_no_oops(self):
368 """If the job raises a user error, there is no oops."""369 """If the job raises a user error, there is no oops."""
370 logging.getLogger().addHandler(OopsHandler('test_runner'))
369 job = RaisingJobUserError('boom')371 job = RaisingJobUserError('boom')
370 runner = JobRunner([job])372 runner = JobRunner([job])
371 runner.runJobHandleError(job)373 runner.runJobHandleError(job)
372 self.assertEqual(0, len(self.oopses))374 self.assertEqual(0, len(self.oopses))
373375
376 def test_runJobHandleErrors_retry_error_no_oops(self):
377 """If the job raises a retry error, there is no oops."""
378 logging.getLogger().addHandler(OopsHandler('test_runner'))
379 job = RaisingRetryJob('completion')
380 runner = JobRunner([job])
381 runner.runJobHandleError(job)
382 self.assertEqual(0, len(self.oopses))
383
374 def test_runJob_raising_retry_error(self):384 def test_runJob_raising_retry_error(self):
375 """If a job raises a retry_error, it should be re-queued."""385 """If a job raises a retry_error, it should be re-queued."""
376 job = RaisingRetryJob('completion')386 job = RaisingRetryJob('completion')
377 runner = JobRunner([job])387 logger = BufferLogger()
388 logger.setLevel(logging.INFO)
389 runner = JobRunner([job], logger=logger)
378 self.assertIs(None, job.scheduled_start)390 self.assertIs(None, job.scheduled_start)
379 with self.expectedLog('Scheduling retry due to RetryError'):391 self.addCleanup(lambda: self.addDetail('log', logger.content))
380 runner.runJob(job, None)392 runner.runJob(job, None)
381 self.assertEqual(JobStatus.WAITING, job.status)393 self.assertEqual(JobStatus.WAITING, job.status)
382 expected_delay = datetime.now(UTC) + timedelta(minutes=10)394 expected_delay = datetime.now(UTC) + timedelta(minutes=10)
383 self.assertThat(395 self.assertThat(
@@ -388,6 +400,8 @@ class TestJobRunner(TestCaseWithFactory):
388 self.assertIsNone(job.lease_expires)400 self.assertIsNone(job.lease_expires)
389 self.assertNotIn(job, runner.completed_jobs)401 self.assertNotIn(job, runner.completed_jobs)
390 self.assertIn(job, runner.incomplete_jobs)402 self.assertIn(job, runner.incomplete_jobs)
403 self.assertIn(
404 'Scheduling retry due to RetryError', logger.getLogBuffer())
391405
392 def test_runJob_exceeding_max_retries(self):406 def test_runJob_exceeding_max_retries(self):
393 """If a job exceeds maximum retries, it should raise normally."""407 """If a job exceeds maximum retries, it should raise normally."""

Subscribers

People subscribed via source and target branches

to status/vote changes: