Merge lp:~hasanammar/conn-check/configs-broker-url into lp:~ubuntuone-hackers/conn-check/configs

Proposed by Hasan Ammar
Status: Merged
Merged at revision: 32
Proposed branch: lp:~hasanammar/conn-check/configs-broker-url
Merge into: lp:~ubuntuone-hackers/conn-check/configs
Diff against target: 131 lines (+97/-1)
3 files modified
Makefile (+1/-1)
conn_check_configs/django.py (+28/-0)
tests.py (+68/-0)
To merge this branch: bzr merge lp:~hasanammar/conn-check/configs-broker-url
Reviewer Review Type Date Requested Status
Guillermo Gonzalez Approve
Maximiliano Bertacchini Approve
Review via email: mp+375725@code.launchpad.net

Commit message

Add support for celery BROKER_URL using kombu parse_url

Description of the change

Add support for celery BROKER_URL using kombu parse_url.

BROKER_URL is the preferred way to specify various broker params (host, port, username, password) instead of specifying them separately (see: https://github.com/ask/celery/commit/8f041c6414feff208d060db17087000b4dda8053).

The additional check in this MP allows specifying a single BROKER_URL, which is then broken down into various bits (host, port, username, password, type) to pass onto conn-check. Breaking the URL into various parts is done using kombu.utils.url.parse_url to ensure the exact same pieces are used for connectivity checking that kombu uses for connectivity.

Minimum kombu version is set to 3.0.30 because it's the earliest version with a working parse_url function.

To post a comment you must log in.
Revision history for this message
Maximiliano Bertacchini (maxiberta) wrote :

Not adding kombu as a hard dependency and instead skipping the check if it's not found is a great idea. +1!

review: Approve
Revision history for this message
Guillermo Gonzalez (verterok) wrote :

nice! +1

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'Makefile'
2--- Makefile 2015-02-26 15:27:23 +0000
3+++ Makefile 2019-11-19 19:48:45 +0000
4@@ -7,7 +7,7 @@
5 virtualenv $(ENV)
6
7 build: $(ENV)
8- $(ENV)/bin/pip install nose pyyaml
9+ $(ENV)/bin/pip install nose pyyaml kombu>=3.0.30
10 $(ENV)/bin/python setup.py develop
11
12 test: $(ENV)
13
14=== modified file 'conn_check_configs/django.py'
15--- conn_check_configs/django.py 2017-05-26 14:54:51 +0000
16+++ conn_check_configs/django.py 2019-11-19 19:48:45 +0000
17@@ -121,6 +121,33 @@
18 return checks
19
20
21+def make_celery_broker_url_checks(settings, options):
22+ checks = []
23+
24+ try:
25+ from kombu.utils.url import parse_url as kombo_parse_url
26+ except ImportError:
27+ pass
28+ else:
29+ broker_url = settings['BROKER_URL'] or settings['CELERY_BROKER_URL']
30+ if broker_url:
31+ url_dict = kombo_parse_url(broker_url)
32+ if url_dict['transport'] in ('amqp', 'redis'):
33+ check = {
34+ 'type': url_dict['transport'],
35+ 'host': url_dict['hostname'],
36+ 'port': url_dict['port'],
37+ 'username': url_dict['userid'],
38+ 'password': url_dict['password'],
39+ }
40+ vhost = url_dict['virtual_host']
41+ if vhost:
42+ check['vhost'] = vhost
43+ checks.append(check)
44+
45+ return checks
46+
47+
48 def make_memcache_checks(settings, options):
49 checks = []
50 caches = settings.get('CACHES', {})
51@@ -178,6 +205,7 @@
52 checks.extend(make_postgres_checks(settings, options))
53 checks.extend(make_oops_checks(settings, options))
54 checks.extend(make_celery_checks(settings, options))
55+ checks.extend(make_celery_broker_url_checks(settings, options))
56 checks.extend(make_memcache_checks(settings, options))
57 checks.extend(make_statsd_checks(settings, options))
58
59
60=== modified file 'tests.py'
61--- tests.py 2014-09-24 11:20:16 +0000
62+++ tests.py 2019-11-19 19:48:45 +0000
63@@ -0,0 +1,68 @@
64+import unittest
65+from collections import defaultdict
66+
67+from conn_check_configs.django import make_celery_broker_url_checks
68+
69+
70+class Celery3BrokerUrlTestCase(unittest.TestCase):
71+ BROKER_URL_SETTING = "BROKER_URL"
72+
73+ VALID_URL_TEST_MAP = {
74+ "amqp://": {
75+ 'type': 'amqp',
76+ 'host': None,
77+ 'port': None,
78+ 'username': None,
79+ 'password': None,
80+ },
81+ "redis://localhost:6379/": {
82+ 'type': 'redis',
83+ 'host': 'localhost',
84+ 'port': 6379,
85+ 'username': None,
86+ 'password': None,
87+ },
88+ "amqp://localhost//foo": {
89+ 'type': 'amqp',
90+ 'host': 'localhost',
91+ 'port': None,
92+ 'username': None,
93+ 'password': None,
94+ 'vhost': '/foo',
95+ },
96+ "amqp://guest:guest@localhost:5672/foo": {
97+ 'type': 'amqp',
98+ 'host': 'localhost',
99+ 'port': 5672,
100+ 'username': 'guest',
101+ 'password': 'guest',
102+ 'vhost': 'foo',
103+ },
104+ }
105+
106+ def test_no_config(self):
107+ conn_checks = make_celery_broker_url_checks(
108+ defaultdict(lambda: None), None
109+ )
110+ self.assertEqual(len(conn_checks), 0)
111+
112+ def test_valid_broker_urls(self):
113+ settings_dict = defaultdict(lambda: None)
114+ for url, expected_result in self.VALID_URL_TEST_MAP.items():
115+ settings_dict[self.BROKER_URL_SETTING] = url
116+ conf_checks = make_celery_broker_url_checks(settings_dict, None)
117+ self.assertDictEqual(conf_checks[0], expected_result)
118+
119+ def test_unsupported_transport(self):
120+ settings_dict = defaultdict(lambda: None)
121+ for transport in (
122+ 'sqs',
123+ 'qpid',
124+ ):
125+ settings_dict[self.BROKER_URL_SETTING] = '%s://' % transport
126+ conf_checks = make_celery_broker_url_checks(settings_dict, None)
127+ self.assertEqual(len(conf_checks), 0)
128+
129+
130+class Celery4BrokerUrlTestCase(Celery3BrokerUrlTestCase):
131+ BROKER_URL_SETTING = "CELERY_BROKER_URL"

Subscribers

People subscribed via source and target branches