Merge lp:~hasanammar/conn-check/amqp-defaults into lp:~ubuntuone-hackers/conn-check/configs

Proposed by Hasan Ammar
Status: Merged
Merged at revision: 33
Proposed branch: lp:~hasanammar/conn-check/amqp-defaults
Merge into: lp:~ubuntuone-hackers/conn-check/configs
Diff against target: 98 lines (+41/-18)
2 files modified
conn_check_configs/django.py (+32/-11)
tests.py (+9/-7)
To merge this branch: bzr merge lp:~hasanammar/conn-check/amqp-defaults
Reviewer Review Type Date Requested Status
Daniel Manrique Approve
Review via email: mp+375985@code.launchpad.net

Commit message

Add default values for AMQP connection

Description of the change

Add default values for AMQP connection to ensure valid values are always passed to conn-check if any are missing.

Default values are taken from: https://github.com/celery/py-amqp/blob/master/amqp/connection.py

To post a comment you must log in.
Revision history for this message
Daniel Manrique (roadmr) wrote :

LGTM

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'conn_check_configs/django.py'
2--- conn_check_configs/django.py 2019-11-19 19:31:10 +0000
3+++ conn_check_configs/django.py 2019-11-25 22:58:33 +0000
4@@ -121,6 +121,37 @@
5 return checks
6
7
8+def _get_broker_checks(url_dict):
9+ # Use default kombu values if the user hasn't provided values for any
10+ # connection setting.
11+ # Default kombu options:
12+ # https://kombu.readthedocs.io/en/master/userguide/connections.html#urls
13+ # Default AMQP values:
14+ # https://github.com/celery/py-amqp/blob/master/amqp/connection.py
15+
16+ conn_type = url_dict['transport']
17+ conn_defaults = {
18+ 'host': 'localhost'
19+ }
20+
21+ if conn_type == 'amqp':
22+ conn_defaults['port'] = 5672
23+ conn_defaults['username'] = 'guest'
24+ conn_defaults['password'] = 'guest'
25+ conn_defaults['vhost'] = '/'
26+ elif conn_type == 'redis':
27+ conn_defaults['port'] = 6379
28+
29+ return {
30+ 'type': conn_type,
31+ 'host': url_dict.get('hostname') or conn_defaults.get('host'),
32+ 'port': url_dict.get('port') or conn_defaults.get('port'),
33+ 'username': url_dict.get('userid') or conn_defaults.get('username'),
34+ 'password': url_dict.get('password') or conn_defaults.get('password'),
35+ 'vhost': url_dict.get('virtual_host') or conn_defaults.get('vhost'),
36+ }
37+
38+
39 def make_celery_broker_url_checks(settings, options):
40 checks = []
41
42@@ -133,17 +164,7 @@
43 if broker_url:
44 url_dict = kombo_parse_url(broker_url)
45 if url_dict['transport'] in ('amqp', 'redis'):
46- check = {
47- 'type': url_dict['transport'],
48- 'host': url_dict['hostname'],
49- 'port': url_dict['port'],
50- 'username': url_dict['userid'],
51- 'password': url_dict['password'],
52- }
53- vhost = url_dict['virtual_host']
54- if vhost:
55- check['vhost'] = vhost
56- checks.append(check)
57+ checks.append(_get_broker_checks(url_dict))
58
59 return checks
60
61
62=== modified file 'tests.py'
63--- tests.py 2019-11-19 19:48:33 +0000
64+++ tests.py 2019-11-25 22:58:33 +0000
65@@ -10,10 +10,11 @@
66 VALID_URL_TEST_MAP = {
67 "amqp://": {
68 'type': 'amqp',
69- 'host': None,
70- 'port': None,
71- 'username': None,
72- 'password': None,
73+ 'host': 'localhost',
74+ 'port': 5672,
75+ 'username': 'guest',
76+ 'password': 'guest',
77+ 'vhost': '/',
78 },
79 "redis://localhost:6379/": {
80 'type': 'redis',
81@@ -21,13 +22,14 @@
82 'port': 6379,
83 'username': None,
84 'password': None,
85+ 'vhost': None,
86 },
87 "amqp://localhost//foo": {
88 'type': 'amqp',
89 'host': 'localhost',
90- 'port': None,
91- 'username': None,
92- 'password': None,
93+ 'port': 5672,
94+ 'username': 'guest',
95+ 'password': 'guest',
96 'vhost': '/foo',
97 },
98 "amqp://guest:guest@localhost:5672/foo": {

Subscribers

People subscribed via source and target branches