openstack/python-troveclient
Code Issues Proposed changes

Fix troveclient to support Mistral

Browse Source 
Mistral gets confused by Trove's (aguably wrong) inclusion
of a member called 'items' in the Pagenated object that
Trove returns as the result of 'list' client methods.

This change changes Pagenated to inherit from the 'list'
class so that the items method is not required (and has
the additional benefit of just generally being a better
implementation of a list type result).

Change-Id: I683120451f69f07f131e6fa422c082f85735b196
Closes-bug: 1585705
Depends-On: Id674ae57bfcdc5e09bde1e323a614b3a03a7cad3
This commit is contained in:
Morgan Jones 2016-05-25 12:29:10 -04:00
parent f7031a7e9a
commit ea0e472edd
8 changed files with 53 additions and 105 deletions

28
troveclient/common.py

@ -40,33 +40,9 @@ def quote_user_host(user, host):
return quoted.replace('.', '%2e')
class Paginated(object):
"""Pretends to be a list if you iterate over it, but also keeps a
next property you can use to get the next page of data.
"""
class Paginated(list):
def __init__(self, items=[], next_marker=None, links=[]):
self.items = items
super(Paginated, self).__init__(items)
self.next = next_marker
self.links = links
def __len__(self):
return len(self.items)
def __iter__(self):
return self.items.__iter__()
def __getitem__(self, key):
return self.items[key]
def __setitem__(self, key, value):
self.items[key] = value
def __delitem__(self, key):
del self.items[key]
def __reversed__(self):
return reversed(self.items)
def __contains__(self, needle):
return needle in self.items

3
troveclient/tests/test_backups.py

@ -87,7 +87,8 @@ class BackupManagerTest(testtools.TestCase):
def test_copy(self):
create_mock = mock.Mock()
self.backups._create = create_mock
args = {'name': 'test_backup', 'backup': '1'}
args = {'name': 'test_backup', 'instance': 'foo',
'backup': '1'}
body = {'backup': args}
self.backups.create(**args)
create_mock.assert_called_with('/backups', body, 'backup')

8
troveclient/tests/test_base.py

@ -289,8 +289,8 @@ class MangerPaginationTests(ManagerTest):
def test_pagination(self):
resp = self.manager._paginated(self.url, self.response_key)
self.manager.api.client.get.assert_called_with(self.url)
self.assertEqual('p1', resp.items[0].foo)
self.assertEqual('p2', resp.items[1].foo)
self.assertEqual('p1', resp[0].foo)
self.assertEqual('p2', resp[1].foo)
self.assertEqual(self.marker, resp.next)
self.assertEqual(self.links, resp.links)
self.assertIsInstance(resp, common.Paginated)
@ -299,8 +299,8 @@ class MangerPaginationTests(ManagerTest):
resp = self.manager._paginated(self.url, self.response_key,
limit=self.limit, marker=self.marker)
self.manager.api.client.get.assert_called_with(self.next_url)
self.assertEqual('p3', resp.items[0].foo)
self.assertEqual('p4', resp.items[1].foo)
self.assertEqual('p3', resp[0].foo)
self.assertEqual('p4', resp[1].foo)
self.assertIsNone(resp.next)
self.assertEqual([], resp.links)
self.assertIsInstance(resp, common.Paginated)

35
troveclient/tests/test_common.py

@ -75,39 +75,6 @@ class PaginatedTest(testtools.TestCase):
super(PaginatedTest, self).tearDown()
def test___init__(self):
self.assertEqual(self.items_, self.pgn.items)
self.assertEqual(self.items_, self.pgn)
self.assertEqual(self.next_marker_, self.pgn.next)
self.assertEqual(self.links_, self.pgn.links)
def test___len__(self):
self.assertEqual(len(self.items_), self.pgn.__len__())
def test___iter__(self):
itr_expected = self.items_.__iter__()
itr = self.pgn.__iter__()
self.assertEqual(next(itr_expected), next(itr))
self.assertEqual(next(itr_expected), next(itr))
self.assertRaises(StopIteration, next, itr_expected)
self.assertRaises(StopIteration, next, itr)
def test___getitem__(self):
self.assertEqual(self.items_[0], self.pgn.__getitem__(0))
def test___setitem__(self):
self.pgn.__setitem__(0, "new-item")
self.assertEqual("new-item", self.pgn.items[0])
def test___delitem(self):
del self.pgn[0]
self.assertEqual(1, self.pgn.__len__())
def test___reversed__(self):
itr = self.pgn.__reversed__()
self.assertEqual("item2", next(itr))
self.assertEqual("item1", next(itr))
self.assertRaises(StopIteration, next, itr)
def test___contains__(self):
self.assertTrue(self.pgn.__contains__("item1"))
self.assertTrue(self.pgn.__contains__("item2"))
self.assertFalse(self.pgn.__contains__("item3"))

4
troveclient/tests/test_instances.py

@ -39,10 +39,6 @@ class InstanceTest(testtools.TestCase):
super(InstanceTest, self).tearDown()
instances.Instance.__init__ = self.orig__init
def test___repr__(self):
self.instance.name = "instance-1"
self.assertEqual('<Instance: instance-1>', self.instance.__repr__())
def test_list_databases(self):
db_list = ['database1', 'database2']
self.instance.manager.databases = mock.Mock()

11
troveclient/v1/backups.py

@ -50,9 +50,16 @@ class Backups(base.ManagerWithFind):
return self._paginated("/backups", "backups", limit, marker,
query_strings)
def create(self, name, instance=None, description=None, parent_id=None,
def create(self, name, instance, description=None, parent_id=None,
backup=None,):
"""Create a new backup from the given instance."""
"""Create a new backup from the given instance.
:param name: name for backup.
:param instance: instance to backup.
:param description: (optional).
:param parent_id: base for incremental backup (optional).
:returns: :class:`Backups`
"""
body = {
"backup": {
"name": name

5
troveclient/v1/instances.py

@ -33,8 +33,6 @@ REBOOT_HARD = 'HARD'
class Instance(base.Resource):
"""An Instance is an opaque instance used to store Database instances."""
def __repr__(self):
return "<Instance: %s>" % self.name
def list_databases(self):
return self.manager.databases.list(self)
@ -186,6 +184,9 @@ class Instances(base.ManagerWithFind):
def backups(self, instance, limit=None, marker=None):
"""Get the list of backups for a specific instance.
:param instance: instance for which to list backups
:param limit: max items to return
:param marker: marker start point
:rtype: list of :class:`Backups`.
"""
url = "/instances/%s/backups" % base.getid(instance)

64
troveclient/v1/shell.py

@ -810,12 +810,12 @@ def do_backup_show(cs, args):
def do_backup_list_instance(cs, args):
"""Lists available backups for an instance."""
instance = _find_instance(cs, args.instance)
wrapper = cs.instances.backups(instance, limit=args.limit,
marker=args.marker)
backups = wrapper.items
while wrapper.next and not args.limit:
wrapper = cs.instances.backups(instance, marker=wrapper.next)
backups += wrapper.items
items = cs.instances.backups(instance, limit=args.limit,
marker=args.marker)
backups = items
while items.next and not args.limit:
items = cs.instances.backups(instance, marker=items.next)
backups += items
utils.print_list(backups, ['id', 'name', 'status',
'parent_id', 'updated'],
order_by='updated')
@ -834,12 +834,12 @@ def do_backup_list_instance(cs, args):
@utils.service_type('database')
def do_backup_list(cs, args):
"""Lists available backups."""
wrapper = cs.backups.list(limit=args.limit, datastore=args.datastore,
marker=args.marker)
backups = wrapper.items
while wrapper.next and not args.limit:
wrapper = cs.backups.list(marker=wrapper.next)
backups += wrapper.items
items = cs.backups.list(limit=args.limit, datastore=args.datastore,
marker=args.marker)
backups = items
while items.next and not args.limit:
items = cs.backups.list(marker=items.next)
backups += items
utils.print_list(backups, ['id', 'instance_id', 'name',
'status', 'parent_id', 'updated'],
order_by='updated')
@ -925,11 +925,11 @@ def do_database_create(cs, args):
def do_database_list(cs, args):
"""Lists available databases on an instance."""
instance = _find_instance(cs, args.instance)
wrapper = cs.databases.list(instance)
databases = wrapper.items
while (wrapper.next):
wrapper = cs.databases.list(instance, marker=wrapper.next)
databases += wrapper.items
items = cs.databases.list(instance)
databases = items
while (items.next):
items = cs.databases.list(instance, marker=items.next)
databases += items
utils.print_list(databases, ['name'])
@ -973,11 +973,11 @@ def do_user_create(cs, args):
def do_user_list(cs, args):
"""Lists the users for an instance."""
instance = _find_instance(cs, args.instance)
wrapper = cs.users.list(instance)
users = wrapper.items
while (wrapper.next):
wrapper = cs.users.list(instance, marker=wrapper.next)
users += wrapper.items
items = cs.users.list(instance)
users = items
while (items.next):
items = cs.users.list(instance, marker=items.next)
users += items
for user in users:
db_names = [db['name'] for db in user.databases]
user.databases = ', '.join(db_names)
@ -1142,11 +1142,11 @@ def do_root_show(cs, args):
@utils.service_type('database')
def do_secgroup_list(cs, args):
"""Lists all security groups."""
wrapper = cs.security_groups.list()
sec_grps = wrapper.items
while (wrapper.next):
wrapper = cs.security_groups.list()
sec_grps += wrapper.items
items = cs.security_groups.list()
sec_grps = items
while (items.next):
items = cs.security_groups.list()
sec_grps += items
utils.print_list(sec_grps, ['id', 'name', 'instance_id'])
@ -1682,13 +1682,13 @@ def do_module_list_instance(cs, args):
def do_module_instances(cs, args):
"""Lists the instances that have a particular module applied."""
module = _find_module(cs, args.module)
wrapper = cs.modules.instances(
items = cs.modules.instances(
module, limit=args.limit, marker=args.marker,
include_clustered=args.include_clustered)
instance_list = wrapper.items
while not args.limit and wrapper.next:
wrapper = cs.modules.instances(module, marker=wrapper.next)
instance_list += wrapper.items
instance_list = items
while not args.limit and items.next:
items = cs.modules.instances(module, marker=items.next)
instance_list += items
_print_instances(instance_list)