Merge lp:~thomir-deactivatedaccount/autopkgtest-result-checker/snappy-image-tests-queues-in-code into lp:autopkgtest-result-checker/snappy-image-tests

Proposed by Thomi Richards
Status: Merged
Approved by: Thomi Richards
Approved revision: 16
Merged at revision: 16
Proposed branch: lp:~thomir-deactivatedaccount/autopkgtest-result-checker/snappy-image-tests-queues-in-code
Merge into: lp:autopkgtest-result-checker/snappy-image-tests
Diff against target: 79 lines (+17/-7)
2 files modified
core_result_checker/__init__.py (+6/-6)
core_result_checker/constants.py (+11/-1)
To merge this branch: bzr merge lp:~thomir-deactivatedaccount/autopkgtest-result-checker/snappy-image-tests-queues-in-code
Reviewer Review Type Date Requested Status
Para Siva (community) Needs Fixing
Thomi Richards (community) Approve
Paul Larson Pending
Celso Providelo Pending
Review via email: mp+259859@code.launchpad.net

This proposal supersedes a proposal from 2015-05-20.

Commit message

Move queue names into constants module.

Description of the change

Move queue names into constants module.

To post a comment you must log in.
Revision history for this message
Paul Larson (pwlars) wrote : Posted in a previous version of this proposal

This looks good. Was this discussed as something we want to move to doing on all the services, or was this specific to the needs of autopkgtest-result-checker?

review: Approve
Revision history for this message
Celso Providelo (cprov) wrote : Posted in a previous version of this proposal

It looks very nice and explores contstants.py to our benefit, because currently we know that most service context changes will imply only in different constants (logic remains the same).

As we see the need to deploy other services in different contexts the their original one, we should probably execute land a similar patch to isolate queue names in constants.py.

[]

review: Approve
Revision history for this message
Para Siva (psivaa) wrote : Posted in a previous version of this proposal

+1, This is a very justifiable middle ground which is even better than the other two extremes.

review: Approve
Revision history for this message
Thomi Richards (thomir-deactivatedaccount) :
review: Approve
Revision history for this message
Para Siva (psivaa) wrote :

It's a little confusing to have the queue names not matching the branch name. I realize it will be more work to change the queue names to include 'snappy' in it.

I'd prefer the name of the branch to lp:autopkgtest-result-checker/core-image-tests or some sort to include 'core' in it.

review: Needs Fixing

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'core_result_checker/__init__.py'
--- core_result_checker/__init__.py 2015-04-07 21:45:33 +0000
+++ core_result_checker/__init__.py 2015-05-21 21:55:04 +0000
@@ -170,7 +170,7 @@
170 if retry_count < self.max_retries:170 if retry_count < self.max_retries:
171 payload['test_run_retry_count'] = retry_count + 1171 payload['test_run_retry_count'] = retry_count + 1
172 self._insert_payload_into_queue(172 self._insert_payload_into_queue(
173 "core.tests.{}".format(constants.API_VERSION),173 constants.TEST_RETRY_QUEUE,
174 payload174 payload
175 )175 )
176 else:176 else:
@@ -180,7 +180,7 @@
180 "letter queue"180 "letter queue"
181 )181 )
182 self._insert_payload_into_queue(182 self._insert_payload_into_queue(
183 "core.deadletters.{}".format(constants.API_VERSION),183 constants.DEAD_LETTER_QUEUE,
184 payload184 payload
185 )185 )
186186
@@ -209,7 +209,7 @@
209 "Requeueing message since swift is not yet consistent"209 "Requeueing message since swift is not yet consistent"
210 )210 )
211 self._insert_payload_into_queue(211 self._insert_payload_into_queue(
212 "core.result.{}".format(constants.API_VERSION),212 constants.INPUT_QUEUE,
213 payload213 payload
214 )214 )
215 else:215 else:
@@ -219,7 +219,7 @@
219 self.swift_retry_seconds219 self.swift_retry_seconds
220 )220 )
221 self._insert_payload_into_queue(221 self._insert_payload_into_queue(
222 "core.deadletters.{}".format(constants.API_VERSION),222 constants.DEAD_LETTER_QUEUE,
223 payload223 payload
224 )224 )
225225
@@ -241,7 +241,7 @@
241 amqp_uris = config.get('amqp', 'uris').split()241 amqp_uris = config.get('amqp', 'uris').split()
242 retry_policy = DefaultRetryPolicy(242 retry_policy = DefaultRetryPolicy(
243 max_retries=3,243 max_retries=3,
244 dead_queue='core.deadletters.{}'.format(constants.API_VERSION)244 dead_queue=constants.DEAD_LETTER_QUEUE
245 )245 )
246 try:246 try:
247 with kombu.Connection(amqp_uris) as connection:247 with kombu.Connection(amqp_uris) as connection:
@@ -250,7 +250,7 @@
250 worker = Worker(swift_manager, retry_publisher)250 worker = Worker(swift_manager, retry_publisher)
251 queue_monitor = SimpleRabbitQueueWorker(251 queue_monitor = SimpleRabbitQueueWorker(
252 connection,252 connection,
253 "core.result.{}".format(constants.API_VERSION),253 constants.INPUT_QUEUE,
254 worker,254 worker,
255 retry_policy,255 retry_policy,
256 )256 )
257257
=== modified file 'core_result_checker/constants.py'
--- core_result_checker/constants.py 2015-04-01 02:02:29 +0000
+++ core_result_checker/constants.py 2015-05-21 21:55:04 +0000
@@ -17,7 +17,17 @@
1717
18"""Constants for this service."""18"""Constants for this service."""
1919
20API_VERSION = "v1"20# Queue names are constant, rather than being defined in the config.
21
22# The queue we listen to for new payloads to check:
23INPUT_QUEUE = "core.result.v1"
24
25# The queue we put fatally error'd payloads into:
26DEAD_LETTER_QUEUE = "core.deadletters.v1"
27
28# The queue we put payloads on to re-run the test:
29TEST_RETRY_QUEUE = "core.tests.v1"
30
2131
22SOLUTION_NAME = "core-image-testing"32SOLUTION_NAME = "core-image-testing"
2333

Subscribers

People subscribed via source and target branches

to all changes: