Merge lp:~cjwatson/launchpad/celery-retry-policy into lp:launchpad

Proposed by Colin Watson
Status: Merged
Merged at revision: 18953
Proposed branch: lp:~cjwatson/launchpad/celery-retry-policy
Merge into: lp:launchpad
Diff against target: 74 lines (+31/-3)
3 files modified
lib/lp/services/job/celeryconfig.py (+8/-0)
lib/lp/services/job/runner.py (+2/-1)
lib/lp/services/job/tests/test_celery.py (+21/-2)
To merge this branch: bzr merge lp:~cjwatson/launchpad/celery-retry-policy
Reviewer Review Type Date Requested Status
William Grant code Approve
Review via email: mp+366757@code.launchpad.net

Commit message

Fix Celery job-running behaviour when RabbitMQ is unconfigured or unresponsive.

Description of the change

I couldn't figure out how to test the first of these two commits in the test suite, but I've tested each of them in sequence on dogfood and it's fixed the problems I was running into there.

To post a comment you must log in.
Revision history for this message
William Grant (wgrant) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/services/job/celeryconfig.py'
2--- lib/lp/services/job/celeryconfig.py 2019-03-09 09:26:01 +0000
3+++ lib/lp/services/job/celeryconfig.py 2019-05-01 12:33:18 +0000
4@@ -78,6 +78,14 @@
5 # fixed once the CELERYD_TASK_SOFT_TIME_LIMIT override is gone.
6 result['worker_concurrency'] = config[queue].concurrency
7
8+ # Don't spend too long failing when RabbitMQ isn't running. We can fall
9+ # back to waiting for the job to be run via cron.
10+ result['broker_transport_options'] = {
11+ 'max_retries': 3,
12+ 'interval_start': 0,
13+ 'interval_step': 0.1,
14+ 'interval_max': 0.1,
15+ }
16 result['broker_url'] = 'amqp://%s:%s@%s/%s' % (
17 config.rabbitmq.userid, config.rabbitmq.password,
18 config.rabbitmq.host, config.rabbitmq.virtual_host)
19
20=== modified file 'lib/lp/services/job/runner.py'
21--- lib/lp/services/job/runner.py 2019-03-09 09:26:01 +0000
22+++ lib/lp/services/job/runner.py 2019-05-01 12:33:18 +0000
23@@ -270,7 +270,8 @@
24
25 def celeryRunOnCommit(self):
26 """Configure transaction so that commit runs this job via Celery."""
27- if not celery_enabled(self.__class__.__name__):
28+ if (config.rabbitmq.host is None or
29+ not celery_enabled(self.__class__.__name__)):
30 return
31 current = transaction.get()
32 current.addAfterCommitHook(self.celeryCommitHook)
33
34=== modified file 'lib/lp/services/job/tests/test_celery.py'
35--- lib/lp/services/job/tests/test_celery.py 2017-06-29 18:06:03 +0000
36+++ lib/lp/services/job/tests/test_celery.py 2019-05-01 12:33:18 +0000
37@@ -1,4 +1,4 @@
38-# Copyright 2012-2017 Canonical Ltd. This software is licensed under the
39+# Copyright 2012-2019 Canonical Ltd. This software is licensed under the
40 # GNU Affero General Public License version 3 (see the file LICENSE).
41
42 """Tests for running jobs via Celery."""
43@@ -33,7 +33,10 @@
44 )
45 from lp.services.job.model.job import Job
46 from lp.services.job.runner import BaseRunnableJob
47-from lp.services.job.tests import block_on_job
48+from lp.services.job.tests import (
49+ block_on_job,
50+ monitor_celery,
51+ )
52 from lp.testing import TestCaseWithFactory
53 from lp.testing.layers import CeleryJobLayer
54
55@@ -202,3 +205,19 @@
56 ]))
57 self.assertEqual(3, job.attempt_count)
58 self.assertEqual(JobStatus.COMPLETED, job.status)
59+
60+ def test_without_rabbitmq(self):
61+ # If no RabbitMQ host is configured, the job is not run via Celery.
62+ self.pushConfig('rabbitmq', host='none')
63+ self.useFixture(FeatureFixture({
64+ 'jobs.celery.enabled_classes': 'TestJob'
65+ }))
66+ with monitor_celery() as responses:
67+ job = TestJob()
68+ job.celeryRunOnCommit()
69+ job_id = job.job_id
70+ transaction.commit()
71+ self.assertEqual([], responses)
72+ store = IStore(Job)
73+ dbjob = store.find(Job, id=job_id)[0]
74+ self.assertEqual(JobStatus.WAITING, dbjob.status)