launchpad-reviewers team mailing list archive

[Aaron Bentley <aaron@xxxxxxxxxxxxx>]

Aaron Bentley has proposed merging lp:~abentley/launchpad/celery-rabbit-config into lp:launchpad with lp:~abentley/launchpad/celery-everywhere as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #973463 in Launchpad itself: "Celery does not use specified virtual host"
  https://bugs.launchpad.net/launchpad/+bug/973463

For more details, see:
https://code.launchpad.net/~abentley/launchpad/celery-rabbit-config/+merge/100813

= Summary =
Fix bug #973463: Celery does not use specified virtual host

== Proposed fix ==
Specify all Launchpad's RabbitMQ settings individually.  Convert to a URL when running celeryd as a subprocess.

== Pre-implementation notes ==
None

== Implementation details ==
URL construction is a pain.  It's easier to use the BROKER_* settings, even though we have to split the port from the host.

We need a URL to pass to the --broker parameter when running celeryd, since the individual settings can't be overridden.  But we can use the connection itself to generate the URL, which should be authoritative.

== Tests ==
bin/test -t Celery

== Demo and Q/A ==
Enable branch scan jobs by setting the "jobs.celery.enabled_classses" feature flag to "BranchScanJob".  Push a branch to qastaging.  Run "sudo rabbitmqctl -p qastaging.launchpad.net list_queues name messages consumers memory".  This should list a "standard" queue with 1 message.

(Restore the jobs.celery.enabled_classes feature flag to its previous value.)


= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/services/job/runner.py
  lib/lp/code/interfaces/branch.py
  lib/lp/services/job/interfaces/job.py
  lib/lp/code/model/tests/test_branchjob.py
  lib/lp/testing/factory.py
  lib/lp/code/model/branch.py
  lib/lp/code/model/tests/test_branchtarget.py
  lib/lp/services/job/celeryconfig.py
  lib/lp/code/model/tests/test_branchpuller.py
  lib/lp/services/job/tests/__init__.py
  lib/lp/code/model/tests/test_branch.py
  lib/lp/code/model/branchjob.py
-- 
https://code.launchpad.net/~abentley/launchpad/celery-rabbit-config/+merge/100813
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/celery-rabbit-config into lp:launchpad.

=== modified file 'lib/lp/services/job/celeryconfig.py'
--- lib/lp/services/job/celeryconfig.py	2012-04-04 15:33:33 +0000
+++ lib/lp/services/job/celeryconfig.py	2012-04-04 15:33:34 +0000
@@ -1,5 +1,10 @@
 from lp.services.config import config
-BROKER_URL = "amqplib://%s" % config.rabbitmq.host
+host, port = config.rabbitmq.host.split(':')
+BROKER_HOST = host
+BROKER_PORT = port
+BROKER_USER = config.rabbitmq.userid
+BROKER_PASSWORD = config.rabbitmq.password
+BROKER_VHOST = config.rabbitmq.virtual_host
 CELERY_IMPORTS = ("lp.services.job.celeryjob", )
 CELERY_RESULT_BACKEND = "amqp"
 CELERY_QUEUES = {

=== modified file 'lib/lp/services/job/tests/__init__.py'
--- lib/lp/services/job/tests/__init__.py	2012-04-04 15:33:33 +0000
+++ lib/lp/services/job/tests/__init__.py	2012-04-04 15:33:34 +0000
@@ -23,9 +23,12 @@
     """
     from lp.services.job.celeryjob import CeleryRunJob
     from lazr.jobrunner.tests.test_celerytask import running
+    # convert config params to a URL, so they can be passed as --broker.
+    with CeleryRunJob.app.broker_connection() as connection:
+        broker_uri = connection.as_uri(include_password=True)
     cmd_args = (
         '--config', 'lp.services.job.celeryconfig',
-        '--broker', CeleryRunJob.app.conf['BROKER_URL'],
+        '--broker', broker_uri,
         '--concurrency', '1',
         '--loglevel', 'INFO',
         '--queues', queue,