From ea0e472edd194243392fd51a5af3a3a77101fddd Mon Sep 17 00:00:00 2001 From: Morgan Jones Date: Wed, 25 May 2016 12:29:10 -0400 Subject: [PATCH] Fix troveclient to support Mistral 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 --- troveclient/common.py | 28 +------------ troveclient/tests/test_backups.py | 3 +- troveclient/tests/test_base.py | 8 ++-- troveclient/tests/test_common.py | 35 +--------------- troveclient/tests/test_instances.py | 4 -- troveclient/v1/backups.py | 11 ++++- troveclient/v1/instances.py | 5 ++- troveclient/v1/shell.py | 64 ++++++++++++++--------------- 8 files changed, 53 insertions(+), 105 deletions(-) diff --git a/troveclient/common.py b/troveclient/common.py index 83133518..8c1efb31 100644 --- a/troveclient/common.py +++ b/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 diff --git a/troveclient/tests/test_backups.py b/troveclient/tests/test_backups.py index a7dd0854..f6401a04 100644 --- a/troveclient/tests/test_backups.py +++ b/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') diff --git a/troveclient/tests/test_base.py b/troveclient/tests/test_base.py index b3597597..f0a5144a 100644 --- a/troveclient/tests/test_base.py +++ b/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) diff --git a/troveclient/tests/test_common.py b/troveclient/tests/test_common.py index 221865e8..9c8ee5ac 100644 --- a/troveclient/tests/test_common.py +++ b/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")) diff --git a/troveclient/tests/test_instances.py b/troveclient/tests/test_instances.py index 1e8e77d2..56dab8e1 100644 --- a/troveclient/tests/test_instances.py +++ b/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('', self.instance.__repr__()) - def test_list_databases(self): db_list = ['database1', 'database2'] self.instance.manager.databases = mock.Mock() diff --git a/troveclient/v1/backups.py b/troveclient/v1/backups.py index d76a3f27..3062f7ff 100644 --- a/troveclient/v1/backups.py +++ b/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 diff --git a/troveclient/v1/instances.py b/troveclient/v1/instances.py index 9fc51c25..83c5834f 100644 --- a/troveclient/v1/instances.py +++ b/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 "" % 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) diff --git a/troveclient/v1/shell.py b/troveclient/v1/shell.py index e9149685..09435b5a 100644 --- a/troveclient/v1/shell.py +++ b/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)