From 93c9bc1fe0ae3f5c95395e7a883fdffcc79d7151 Mon Sep 17 00:00:00 2001 From: Flavio Percoco Date: Tue, 27 Jan 2015 14:20:27 +0100 Subject: [PATCH] Add a `--limit` parameter to list operations In v2, we use the link header during paginations to know when there are more images available in the server that could be returned in the same request. This way, it's possible to iterate over the generator returned by list and consume the images in the server. However, it's currently not possible to tell glanceclient the exact number of images we want, which basically means that it'll *always* go through the whole list of images in the server unless the limit is implemented by the consumer. DocImpact Change-Id: I9f65a40d4eafda6320e5c7d94d03fcd1bbc93e33 Closes-bug: #1415035 --- glanceclient/v2/images.py | 31 +++++++++++++++++++++-------- glanceclient/v2/shell.py | 4 ++++ tests/v2/test_images.py | 41 +++++++++++++++++++++++++++++++++++++++ tests/v2/test_shell_v2.py | 2 ++ 4 files changed, 70 insertions(+), 8 deletions(-) diff --git a/glanceclient/v2/images.py b/glanceclient/v2/images.py index b49c5ee0..5ed7c681 100644 --- a/glanceclient/v2/images.py +++ b/glanceclient/v2/images.py @@ -46,7 +46,19 @@ class Controller(object): ori_validate_fun = self.model.validate empty_fun = lambda *args, **kwargs: None - def paginate(url): + limit = kwargs.get('limit') + # NOTE(flaper87): Don't use `get('page_size', DEFAULT_SIZE)` otherwise, + # it could be possible to send invalid data to the server by passing + # page_size=None. + page_size = kwargs.get('page_size') or DEFAULT_PAGE_SIZE + + def paginate(url, page_size, limit=None): + + if limit and page_size > limit: + # NOTE(flaper87): Avoid requesting 2000 images when limit is 1 + url = url.replace("limit=%s" % page_size, + "limit=%s" % limit) + resp, body = self.http_client.get(url) for image in body['images']: # NOTE(bcwaldon): remove 'self' for now until we have @@ -60,6 +72,11 @@ class Controller(object): # image entry for each page. self.model.validate = empty_fun + if limit: + limit -= 1 + if limit <= 0: + raise StopIteration + # NOTE(zhiyan); Reset validation function. self.model.validate = ori_validate_fun @@ -68,15 +85,13 @@ class Controller(object): except KeyError: return else: - for image in paginate(next_url): + for image in paginate(next_url, page_size, limit): yield image filters = kwargs.get('filters', {}) - - if not kwargs.get('page_size'): - filters['limit'] = DEFAULT_PAGE_SIZE - else: - filters['limit'] = kwargs['page_size'] + # NOTE(flaper87): We paginate in the client, hence we use + # the page_size as Glance's limit. + filters['limit'] = page_size tags = filters.pop('tag', []) tags_url_params = [] @@ -94,7 +109,7 @@ class Controller(object): for param in tags_url_params: url = '%s&%s' % (url, parse.urlencode(param)) - for image in paginate(url): + for image in paginate(url, page_size, limit): yield image def get(self, image_id): diff --git a/glanceclient/v2/shell.py b/glanceclient/v2/shell.py index 4da2d990..a49d2919 100644 --- a/glanceclient/v2/shell.py +++ b/glanceclient/v2/shell.py @@ -107,6 +107,8 @@ def do_image_update(gc, args): utils.print_image(image) +@utils.arg('--limit', metavar='', default=None, type=int, + help='Maximum number of images to get.') @utils.arg('--page-size', metavar='', default=None, type=int, help='Number of images to request in each paginated request.') @utils.arg('--visibility', metavar='', @@ -135,6 +137,8 @@ def do_image_list(gc, args): filters = dict([item for item in filter_items if item[1] is not None]) kwargs = {'filters': filters} + if args.limit is not None: + kwargs['limit'] = args.page_size if args.page_size is not None: kwargs['page_size'] = args.page_size diff --git a/tests/v2/test_images.py b/tests/v2/test_images.py index ed6e75f1..2f3efeb0 100644 --- a/tests/v2/test_images.py +++ b/tests/v2/test_images.py @@ -78,6 +78,25 @@ data_fixtures = { ]}, ), }, + '/v2/images?limit=2': { + 'GET': ( + {}, + { + 'images': [ + { + 'id': '3a4560a1-e585-443e-9b39-553b46ec92d1', + 'name': 'image-1', + }, + { + 'id': '6f99bf80-2ee6-47cf-acfe-1f1fabb7e810', + 'name': 'image-2', + }, + ], + 'next': ('/v2/images?limit=2&' + 'marker=6f99bf80-2ee6-47cf-acfe-1f1fabb7e810'), + }, + ), + }, '/v2/images?limit=1': { 'GET': ( {}, @@ -104,6 +123,17 @@ data_fixtures = { ]}, ), }, + ('/v2/images?limit=1&marker=6f99bf80-2ee6-47cf-acfe-1f1fabb7e810'): { + 'GET': ( + {}, + {'images': [ + { + 'id': '3f99bf80-2ee6-47cf-acfe-1f1fabb7e811', + 'name': 'image-3', + }, + ]}, + ), + }, '/v2/images/3a4560a1-e585-443e-9b39-553b46ec92d1': { 'GET': ( {}, @@ -420,6 +450,17 @@ class TestController(testtools.TestCase): self.assertEqual('6f99bf80-2ee6-47cf-acfe-1f1fabb7e810', images[1].id) self.assertEqual('image-2', images[1].name) + def test_list_images_paginated_with_limit(self): + # NOTE(bcwaldon):cast to list since the controller returns a generator + images = list(self.controller.list(limit=3, page_size=2)) + self.assertEqual('3a4560a1-e585-443e-9b39-553b46ec92d1', images[0].id) + self.assertEqual('image-1', images[0].name) + self.assertEqual('6f99bf80-2ee6-47cf-acfe-1f1fabb7e810', images[1].id) + self.assertEqual('image-2', images[1].name) + self.assertEqual('3f99bf80-2ee6-47cf-acfe-1f1fabb7e811', images[2].id) + self.assertEqual('image-3', images[2].name) + self.assertEqual(3, len(images)) + def test_list_images_visibility_public(self): filters = {'filters': {'visibility': 'public'}} images = list(self.controller.list(**filters)) diff --git a/tests/v2/test_shell_v2.py b/tests/v2/test_shell_v2.py index 3480e5c9..fd781c0b 100644 --- a/tests/v2/test_shell_v2.py +++ b/tests/v2/test_shell_v2.py @@ -61,6 +61,7 @@ class ShellV2Test(testtools.TestCase): def test_do_image_list(self): input = { + 'limit': None, 'page_size': 18, 'visibility': True, 'member_status': 'Fake', @@ -88,6 +89,7 @@ class ShellV2Test(testtools.TestCase): def test_do_image_list_with_property_filter(self): input = { + 'limit': None, 'page_size': 1, 'visibility': True, 'member_status': 'Fake',