Python client library for Zaqar

Subscriptions listing is not written good and causes many potential and real problems.

Bug #1538366 reported by Eva Balycheva
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Python client library for Zaqar
Fix Released
Medium
Eva Balycheva

Bug Description

Problem 1(core problem):

See this: https://github.com/openstack/python-zaqarclient/blob/master/zaqarclient/queues/v2/client.py#L79

I think this is not good, let's not modify response from Zaqar.
The queue name information is already under the key 'source'. There's no need for adding duplicate 'queue_name' key.

This can add big confusion to the other developers, for example for the creator of the https://review.openstack.org/#/c/269398/ patch. It also makes it possible to write not very good unit tests.

Problem 2:

This problem is only possible because of problem 1.

See this function: https://github.com/openstack/python-zaqarclient/blob/master/zaqarclient/queues/v2/subscription.py#L78

I think it's written incorrectly, it just made to work with the code listed in Problem 1.

When the function is used by https://review.openstack.org/#/c/269398/ patch where response from Zaqar is not modified (which is sane), the function causes an exception. See the log:

Traceback (most recent call last):
  File "/home/yell/Sync/Repos/OHMY!/ohmy.py", line 105, in <module>
    list_subscriptions()
  File "/home/yell/Sync/Repos/OHMY!/ohmy.py", line 80, in list_subscriptions
    for sub in queue.subscriptions():
  File "/home/yell/Sync/Repos/python-zaqarclient/zaqarclient/queues/v1/iterator.py", line 106, in __next__
    return self._create_function(args)
  File "/home/yell/Sync/Repos/python-zaqarclient/zaqarclient/queues/v2/subscription.py", line 79, in <lambda>
    return lambda kwargs: Subscription(parent, kwargs.pop("queue_name"),
KeyError: 'queue_name'

Problem 3:

This problem is only possible because of problem 1.

See: https://github.com/openstack/python-zaqarclient/blob/master/zaqarclient/tests/queues/subscriptions.py#L123

This response body from Zaqar does not represent real response which is bad. It is missing 'source' key-value pair for each subscription in the body.

See original description
Eva Balycheva (ubershy)
description: updated
Changed in python-zaqarclient:
assignee: nobody → Eva Balycheva (ubershy)
Eva Balycheva (ubershy)
description: updated
Eva Balycheva (ubershy)
description: updated
Eva Balycheva (ubershy)
description: updated
summary: - Subscriptions listing does not work
+ Subscriptions listing is not written good and causes many potential and
+ real problems.
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to python-zaqarclient (master)

Fix proposed to branch: master
Review: https://review.openstack.org/272909

Changed in python-zaqarclient:
status: New → In Progress
Feilong Wang (flwang)
Changed in python-zaqarclient:
importance: Undecided → Medium
status: In Progress → Confirmed
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to python-zaqarclient (master)

Reviewed: https://review.openstack.org/272909
Committed: https://git.openstack.org/cgit/openstack/python-zaqarclient/commit/?id=f24a7e9fa32feb4077883f7a10180d110ee6edc0
Submitter: Jenkins
Branch: master

commit f24a7e9fa32feb4077883f7a10180d110ee6edc0
Author: Eva Balycheva <email address hidden>
Date: Wed Jan 27 08:03:53 2016 +0300

    Improve subscription listing

    The way subscriptions are listed by Client.subscriptions method is not
    very good. Besides doing nice work, this method modifies subscription
    list response body from Zaqar by adding redundant 'queue_name' key to
    each subscription dictionary in the response body, while there is
    already 'source' key containing queue name.

    Because of that it's possible to write a unit test, where queue list
    response body from Zaqar does not represent real response that could
    come from Zaqar. As example see
    zaqarclient.tests.queues.subscriptions.QueuesV2SubscriptionUnitTest.
    test_subscription_list. This test is missing 'source' keys in response
    body.

    This patch makes subscription listing code stop modifying responses from
    Zaqar. As expected, the example unit test fails after that. This patch
    fixes the test also and checks more subscription object's fields.

    Also this patch improves subscription listing functional test to check
    all fields of subscription object that was deserialized from Zaqar
    response, i.e. checks zaqarclient.queues.v2.subscription.create_object
    method. It is also needed, because before this patch, subscription's
    queue name field wasn't checked by any of the tests, making a recent bug
    like https://bugs.launchpad.net/python-zaqarclient/+bug/1538367
    possible.

    Change-Id: Iae5bffb296efd655a34584c7f2162adbe6d029e2
    Closes-Bug: 1538366

Changed in python-zaqarclient:
status: Confirmed → Fix Released
Revision history for this message
Doug Hellmann (doug-hellmann) wrote : Fix included in openstack/python-zaqarclient 1.0.0

This issue was fixed in the openstack/python-zaqarclient 1.0.0 release.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

This issue was fixed in the openstack/python-zaqarclient 1.0.0 release.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.