From 8eaf8139842227c106a978b9644630acafe3b6ab Mon Sep 17 00:00:00 2001 From: Eva Balycheva Date: Sun, 7 Feb 2016 10:02:12 +0300 Subject: [PATCH] Do not catch ConflictError on subscription create Currently 'Subscription' class catches ConflictError inside and prints text 'The subscriber has been existed already.'. This is not good for two reasons: 1. The library user(program) wants to handle this exception by himself using convenient try/except mechanism along with many other exceptions that can happen during subscription creation. 2. The library user might not want some text to be printed in this situation by python-zaqarclient. For now this is the only situation when python-zaqarclient prints something when it is used as library. This patch makes "Subscription" class not to handle ConflictError exception by itself. And creates functional and unit tests to assert that this behavior is raised, when Zaqar returns 409 response code on subscription creation. In the future it would be good to list all exceptions that can be raised by the "Client" class's methods in their docstrings. See "zaqarclient/queues/v1/client.py" and "zaqarclient/queues/v2/client.py". Also it would be good if "Pool" and "Flavor" classes would throw ConflictError exception too. Closes-Bug: 1542780 Change-Id: I17c0e32a3dabec8303b3055a0d97db857d947b34 --- zaqarclient/queues/v2/subscription.py | 15 +++++---------- zaqarclient/tests/queues/subscriptions.py | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/zaqarclient/queues/v2/subscription.py b/zaqarclient/queues/v2/subscription.py index 82ff2382..88701866 100644 --- a/zaqarclient/queues/v2/subscription.py +++ b/zaqarclient/queues/v2/subscription.py @@ -14,7 +14,6 @@ # limitations under the License. from zaqarclient.queues.v2 import core -from zaqarclient.transport import errors class Subscription(object): @@ -45,16 +44,12 @@ class Subscription(object): 'ttl': self.ttl, 'options': self.options } - try: - subscription = core.subscription_create(trans, req, - self.queue_name, - subscription_data) + subscription = core.subscription_create(trans, req, + self.queue_name, + subscription_data) - if subscription and 'subscription_id' in subscription: - self.id = subscription['subscription_id'] - except errors.ConflictError: - # ConflictError means the subscription already exists. - print('The subscriber has been existed already.') + if subscription and 'subscription_id' in subscription: + self.id = subscription['subscription_id'] if self.id: sub = core.subscription_get(trans, req, self.queue_name, self.id) diff --git a/zaqarclient/tests/queues/subscriptions.py b/zaqarclient/tests/queues/subscriptions.py index eb6239f4..f0701b0f 100644 --- a/zaqarclient/tests/queues/subscriptions.py +++ b/zaqarclient/tests/queues/subscriptions.py @@ -46,6 +46,19 @@ class QueuesV2SubscriptionUnitTest(base.QueuesTestBase): self.assertEqual(3600, subscription.ttl) self.assertEqual('fake_id', subscription.id) + def test_subscription_create_duplicate_throws_conflicterror(self): + subscription_data = {'subscriber': 'http://trigger.me', + 'ttl': 3600} + + with mock.patch.object(self.transport, 'send', + autospec=True) as send_method: + + create_resp = response.Response(None, None, status_code=409) + send_method.return_value = create_resp + + self.assertRaises(errors.ConflictError, self.client.subscription, + 'beijing', **subscription_data) + def test_subscription_update(self): subscription_data = {'subscriber': 'http://trigger.me', 'ttl': 3600} @@ -172,6 +185,11 @@ class QueuesV2SubscriptionFunctionalTest(base.QueuesTestBase): self.assertEqual('http://trigger.he', self.subscription_2.subscriber) self.assertEqual(7200, self.subscription_2.ttl) + def test_subscription_create_duplicate_throws_conflicterror(self): + subscription_data_1 = {'subscriber': 'http://trigger.me', 'ttl': 3600} + self.assertRaises(errors.ConflictError, self.client.subscription, + 'beijing', **subscription_data_1) + def test_subscription_update(self): sub = self.client.subscription(self.queue_name, auto_create=False, **{'id': self.subscription_1.id})