openstack/python-zaqarclient
Code Issues Proposed changes

Do not catch ConflictError on subscription create

Browse Source 
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
This commit is contained in:
Eva Balycheva 2016-02-07 10:02:12 +03:00
parent ae5498a8ca
commit 8eaf813984
2 changed files with 23 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
zaqarclient/queues/v2/subscription.py
View File

@ -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)

18
zaqarclient/tests/queues/subscriptions.py
View File

@ -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})