From 61a1476856504c90cbd0b6ef95e5693f4db6addc Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Sun, 23 Oct 2016 22:43:48 -0700 Subject: [PATCH 1/8] Adding a pages iterator side-by-side with items iterator. --- core/google/cloud/iterator.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/core/google/cloud/iterator.py b/core/google/cloud/iterator.py index 06242e57597d..715e280ac54a 100644 --- a/core/google/cloud/iterator.py +++ b/core/google/cloud/iterator.py @@ -274,6 +274,31 @@ def _verify_params(self): raise ValueError('Using a reserved parameter', reserved_in_use) + def _pages_iter(self): + """Generator of pages of API responses. + + Yields :class:`Page` instances. + """ + while self._has_next_page(): + response = self._get_next_page_response() + page = Page(self, response, self._items_key, + self._item_to_value) + self._page_start(self, page, response) + yield page + + @property + def pages(self): + """Iterator of pages in the response. + + :rtype: :class:`~types.GeneratorType` + :returns: A generator of :class:`Page` instances. + :raises ValueError: If the iterator has already been started. + """ + if self._started: + raise ValueError('Iterator has already started', self) + self._started = True + return self._pages_iter() + @property def page(self): """The current page of results that has been retrieved. From 1d38abbfc4069e30cafc3c75f01a50b69d95cc7e Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Sun, 23 Oct 2016 22:47:12 -0700 Subject: [PATCH 2/8] Using the pages iterator in the main iterator. NOTE: There is a current mismatch between incrementing Iterator.num_results in Iterator.__iter__ vs. incrementing it in Iterator.pages (there it won't be incremented, so this will need to be addressed in a subsequent commit). --- core/google/cloud/iterator.py | 29 +++++++---------------------- 1 file changed, 7 insertions(+), 22 deletions(-) diff --git a/core/google/cloud/iterator.py b/core/google/cloud/iterator.py index 715e280ac54a..1cc0f7a0842d 100644 --- a/core/google/cloud/iterator.py +++ b/core/google/cloud/iterator.py @@ -314,16 +314,13 @@ def page(self): return self._page def __iter__(self): - """The :class:`Iterator` is an iterator. - - :rtype: :class:`Iterator` - :returns: Current instance. - :raises ValueError: If the iterator has already been started. - """ - if self._started: - raise ValueError('Iterator has already started', self) - self._started = True - return self + """Iterator for each item returned.""" + # NOTE: We don't check if the iterator has started since the pages + # iterator already does this. + for page in self.pages: + for item in page: + self.num_results += 1 + yield item def update_page(self, require_empty=True): """Move to the next page in the result set. @@ -364,18 +361,6 @@ def update_page(self, require_empty=True): msg = _PAGE_ERR_TEMPLATE % (self._page, self.page.remaining) raise ValueError(msg) - def next(self): - """Get the next item from the request.""" - self.update_page(require_empty=False) - if self.page is None: - raise StopIteration - item = six.next(self.page) - self.num_results += 1 - return item - - # Alias needed for Python 2/3 support. - __next__ = next - def _has_next_page(self): """Determines whether or not there are more pages with results. From fbdd73aa36e85c787fa399398c35da0610fe41e6 Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Sun, 23 Oct 2016 22:49:35 -0700 Subject: [PATCH 3/8] Remove some manual paging functionality. --- core/google/cloud/iterator.py | 55 ----------------------------------- 1 file changed, 55 deletions(-) diff --git a/core/google/cloud/iterator.py b/core/google/cloud/iterator.py index 1cc0f7a0842d..98a85d613586 100644 --- a/core/google/cloud/iterator.py +++ b/core/google/cloud/iterator.py @@ -107,7 +107,6 @@ import six -_UNSET = object() _NO_MORE_PAGES_ERR = 'Iterator has no more pages.' _UNSTARTED_ERR = ( 'Iterator has not been started. Either begin iterating, ' @@ -261,7 +260,6 @@ def __init__(self, client, path, item_to_value, self.page_number = 0 self.next_page_token = page_token self.num_results = 0 - self._page = _UNSET def _verify_params(self): """Verifies the parameters don't use any reserved parameter. @@ -299,20 +297,6 @@ def pages(self): self._started = True return self._pages_iter() - @property - def page(self): - """The current page of results that has been retrieved. - - If there are no more results, will return :data:`None`. - - :rtype: :class:`Page` - :returns: The page of items that has been retrieved. - :raises AttributeError: If the page has not been set. - """ - if self._page is _UNSET: - raise AttributeError(_UNSTARTED_ERR) - return self._page - def __iter__(self): """Iterator for each item returned.""" # NOTE: We don't check if the iterator has started since the pages @@ -322,45 +306,6 @@ def __iter__(self): self.num_results += 1 yield item - def update_page(self, require_empty=True): - """Move to the next page in the result set. - - If the current page is not empty and ``require_empty`` is :data:`True` - then an exception will be raised. If the current page is not empty - and ``require_empty`` is :data:`False`, then this will return - without updating the current page. - - If the current page **is** empty, but there are no more results, - sets the current page to :data:`None`. - - If there are no more pages, throws an exception. - - :type require_empty: bool - :param require_empty: (Optional) Flag to indicate if the current page - must be empty before updating. - - :raises ValueError: If ``require_empty`` is :data:`True` but the - current page is not empty. - :raises ValueError: If there are no more pages. - """ - if self._page is None: - raise ValueError(_NO_MORE_PAGES_ERR) - - # NOTE: This assumes Page.remaining can never go below 0. - page_empty = self._page is _UNSET or self._page.remaining == 0 - if page_empty: - if self._has_next_page(): - response = self._get_next_page_response() - self._page = Page(self, response, self._items_key, - self._item_to_value) - self._page_start(self, self._page, response) - else: - self._page = None - else: - if require_empty: - msg = _PAGE_ERR_TEMPLATE % (self._page, self.page.remaining) - raise ValueError(msg) - def _has_next_page(self): """Determines whether or not there are more pages with results. From d349afb2abfa8812467c2ccace208e8ce87d3ebc Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Sun, 23 Oct 2016 22:51:45 -0700 Subject: [PATCH 4/8] Properly tracking the Iterator #results in pages/items iterators. --- core/google/cloud/iterator.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/google/cloud/iterator.py b/core/google/cloud/iterator.py index 98a85d613586..1e575a999544 100644 --- a/core/google/cloud/iterator.py +++ b/core/google/cloud/iterator.py @@ -282,6 +282,7 @@ def _pages_iter(self): page = Page(self, response, self._items_key, self._item_to_value) self._page_start(self, page, response) + self.num_results += page.num_items yield page @property @@ -302,6 +303,9 @@ def __iter__(self): # NOTE: We don't check if the iterator has started since the pages # iterator already does this. for page in self.pages: + # Decrement the total results since the pages iterator adds + # to it when each page is encountered. + self.num_results -= page.num_items for item in page: self.num_results += 1 yield item From 8f81dcce0c9d308015b101f31c600e79bb733736 Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Sun, 23 Oct 2016 23:03:05 -0700 Subject: [PATCH 5/8] Updating Iterator/paging documentation. --- core/google/cloud/iterator.py | 76 ++++++++++++----------------------- 1 file changed, 25 insertions(+), 51 deletions(-) diff --git a/core/google/cloud/iterator.py b/core/google/cloud/iterator.py index 1e575a999544..f9c818b56ca9 100644 --- a/core/google/cloud/iterator.py +++ b/core/google/cloud/iterator.py @@ -43,64 +43,38 @@ ... break When iterating, not every new item will send a request to the server. -To monitor these requests, track the current page of the iterator:: +To iterate based on each page of items (where a page corresponds to +a request):: >>> iterator = Iterator(...) - >>> iterator.page_number - 0 - >>> next(iterator) - - >>> iterator.page_number - 1 - >>> iterator.page.remaining - 1 - >>> next(iterator) - - >>> iterator.page_number - 1 - >>> iterator.page.remaining - 0 - >>> next(iterator) - - >>> iterator.page_number - 2 - >>> iterator.page.remaining - 19 - -It's also possible to consume an entire page and handle the paging process -manually:: - - >>> iterator = Iterator(...) - >>> # Manually pull down the first page. - >>> iterator.update_page() - >>> items = list(iterator.page) - >>> items + >>> for page in iterator.pages: + ... print('=' * 20) + ... print(' Page number: %d' % (iterator.page_number,)) + ... print(' Items in page: %d' % (page.num_items,)) + ... print(' First item: %r' % (next(page),)) + ... print('Items remaining: %d' % (page.remaining,)) + ... print('Next page token: %s' % (iterator.next_page_token,)) + ==================== + Page number: 1 + Items in page: 1 + First item: + Items remaining: 0 + Next page token: eav1OzQB0OM8rLdGXOEsyQWSG + ==================== + Page number: 2 + Items in page: 19 + First item: + Items remaining: 18 + Next page token: None + +To consume an entire page:: + + >>> list(page) [ , , , ] - >>> iterator.page.remaining - 0 - >>> iterator.page.num_items - 3 - >>> iterator.next_page_token - 'eav1OzQB0OM8rLdGXOEsyQWSG' - >>> - >>> # Ask for the next page to be grabbed. - >>> iterator.update_page() - >>> list(iterator.page) - [ - , - , - ] - >>> - >>> # When there are no more results - >>> iterator.next_page_token is None - True - >>> iterator.update_page() - >>> iterator.page is None - True """ From 57a3c49d5f50a0af34cd93b2350e4fd6919c051b Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Sun, 23 Oct 2016 23:32:02 -0700 Subject: [PATCH 6/8] Updating core unit tests for Iterator changes. Also moving __iter__ functionality into a helper so that the "started?" check could be done **before** entering the generator. This is because the "self.pages" generator wouldn't be entered until an item was consumed off the items iterator. --- core/google/cloud/iterator.py | 25 ++++---- core/unit_tests/test_iterator.py | 107 ++++++++++++++----------------- 2 files changed, 63 insertions(+), 69 deletions(-) diff --git a/core/google/cloud/iterator.py b/core/google/cloud/iterator.py index f9c818b56ca9..078593e72539 100644 --- a/core/google/cloud/iterator.py +++ b/core/google/cloud/iterator.py @@ -81,13 +81,6 @@ import six -_NO_MORE_PAGES_ERR = 'Iterator has no more pages.' -_UNSTARTED_ERR = ( - 'Iterator has not been started. Either begin iterating, ' - 'call next(my_iter) or call my_iter.update_page().') -_PAGE_ERR_TEMPLATE = ( - 'Tried to update the page while current page (%r) still has %d ' - 'items remaining.') DEFAULT_ITEMS_KEY = 'items' """The dictionary key used to retrieve items from each response.""" @@ -272,11 +265,9 @@ def pages(self): self._started = True return self._pages_iter() - def __iter__(self): + def _items_iter(self): """Iterator for each item returned.""" - # NOTE: We don't check if the iterator has started since the pages - # iterator already does this. - for page in self.pages: + for page in self._pages_iter(): # Decrement the total results since the pages iterator adds # to it when each page is encountered. self.num_results -= page.num_items @@ -284,6 +275,18 @@ def __iter__(self): self.num_results += 1 yield item + def __iter__(self): + """Iterator for each item returned. + + :rtype: :class:`~types.GeneratorType` + :returns: A generator of items from the API. + :raises ValueError: If the iterator has already been started. + """ + if self._started: + raise ValueError('Iterator has already started', self) + self._started = True + return self._items_iter() + def _has_next_page(self): """Determines whether or not there are more pages with results. diff --git a/core/unit_tests/test_iterator.py b/core/unit_tests/test_iterator.py index e16edaaa00fa..53613550323b 100644 --- a/core/unit_tests/test_iterator.py +++ b/core/unit_tests/test_iterator.py @@ -102,7 +102,6 @@ def _makeOne(self, *args, **kw): def test_constructor(self): from google.cloud.iterator import _do_nothing_page_start - from google.cloud.iterator import _UNSET connection = _Connection() client = _Client(connection) @@ -120,7 +119,6 @@ def test_constructor(self): self.assertEqual(iterator.page_number, 0) self.assertIsNone(iterator.next_page_token) self.assertEqual(iterator.num_results, 0) - self.assertIs(iterator._page, _UNSET) def test_constructor_w_extra_param_collision(self): connection = _Connection() @@ -130,61 +128,15 @@ def test_constructor_w_extra_param_collision(self): with self.assertRaises(ValueError): self._makeOne(client, path, None, extra_params=extra_params) - def test_page_property(self): - iterator = self._makeOne(None, None, None) - page = object() - iterator._page = page - self.assertIs(iterator.page, page) - - def test_page_property_unset(self): - from google.cloud.iterator import _UNSET - - iterator = self._makeOne(None, None, None) - self.assertIs(iterator._page, _UNSET) - with self.assertRaises(AttributeError): - getattr(iterator, 'page') - - def test_update_page_no_more(self): - iterator = self._makeOne(None, None, None) - iterator._page = None - with self.assertRaises(ValueError): - iterator.update_page() - - def test_update_page_not_empty_success(self): - from google.cloud.iterator import Page - - iterator = self._makeOne(None, None, None) - page = Page(None, {}, '', None) - iterator._page = page - iterator._page._remaining = 1 - iterator.update_page(require_empty=False) - self.assertIs(iterator._page, page) - - def test_update_page_not_empty_fail(self): - from google.cloud.iterator import Page - - iterator = self._makeOne(None, None, None) - iterator._page = Page(None, {}, '', None) - iterator._page._remaining = 1 - with self.assertRaises(ValueError): - iterator.update_page(require_empty=True) - - def test_update_page_empty_then_no_more(self): - iterator = self._makeOne(None, None, None) - # Fake that there are no more pages. - iterator.page_number = 1 - iterator.next_page_token = None - iterator.update_page() - self.assertIsNone(iterator.page) - - def test_update_page_empty_then_another(self): + def test_pages_iter_empty_then_another(self): + import six from google.cloud._testing import _Monkey from google.cloud import iterator as MUT items_key = 'its-key' iterator = self._makeOne(None, None, None, items_key=items_key) # Fake the next page class. - fake_page = object() + fake_page = MUT.Page(None, {}, '', None) page_args = [] def dummy_response(): @@ -195,20 +147,58 @@ def dummy_page_class(*args): return fake_page iterator._get_next_page_response = dummy_response + pages_iter = iterator.pages with _Monkey(MUT, Page=dummy_page_class): - iterator.update_page() - self.assertIs(iterator.page, fake_page) + page = six.next(pages_iter) + self.assertIs(page, fake_page) self.assertEqual( page_args, [(iterator, {}, items_key, iterator._item_to_value)]) + def test_pages_property(self): + import types + + iterator = self._makeOne(None, None, None) + self.assertIsInstance(iterator.pages, types.GeneratorType) + + def test_pages_property_started(self): + import types + + iterator = self._makeOne(None, None, None) + pages_iter = iterator.pages + self.assertIsInstance(pages_iter, types.GeneratorType) + with self.assertRaises(ValueError): + getattr(iterator, 'pages') + + def test_pages_property_items_started(self): + import types + + iterator = self._makeOne(None, None, None) + items_iter = iter(iterator) + self.assertIsInstance(items_iter, types.GeneratorType) + with self.assertRaises(ValueError): + getattr(iterator, 'pages') + def test___iter__(self): + import types + iterator = self._makeOne(None, None, None) - self.assertIs(iter(iterator), iterator) + self.assertIsInstance(iter(iterator), types.GeneratorType) def test___iter___started(self): + import types + iterator = self._makeOne(None, None, None) iter_obj = iter(iterator) - self.assertIs(iter_obj, iterator) + self.assertIsInstance(iter_obj, types.GeneratorType) + with self.assertRaises(ValueError): + iter(iterator) + + def test___iter___pages_started(self): + import types + + iterator = self._makeOne(None, None, None) + pages_iter = iterator.pages + self.assertIsInstance(pages_iter, types.GeneratorType) with self.assertRaises(ValueError): iter(iterator) @@ -231,16 +221,17 @@ def item_to_value(iterator, item): # pylint: disable=unused-argument item_to_value=item_to_value) self.assertEqual(iterator.num_results, 0) - val1 = six.next(iterator) + items_iter = iter(iterator) + val1 = six.next(items_iter) self.assertEqual(val1, item1) self.assertEqual(iterator.num_results, 1) - val2 = six.next(iterator) + val2 = six.next(items_iter) self.assertEqual(val2, item2) self.assertEqual(iterator.num_results, 2) with self.assertRaises(StopIteration): - six.next(iterator) + six.next(items_iter) kw, = connection._requested self.assertEqual(kw['method'], 'GET') From a6890c57691dffd45b5c745fe197844e503e2d01 Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Mon, 24 Oct 2016 00:11:51 -0700 Subject: [PATCH 7/8] Fixing unit tests for Iterator dependent packages. --- bigquery/unit_tests/test_client.py | 37 ++++++++++++++-------- dns/unit_tests/test_client.py | 10 +++--- dns/unit_tests/test_zone.py | 20 +++++++----- resource_manager/unit_tests/test_client.py | 6 ++-- storage/unit_tests/test_bucket.py | 16 +++++----- storage/unit_tests/test_client.py | 6 ++-- 6 files changed, 55 insertions(+), 40 deletions(-) diff --git a/bigquery/unit_tests/test_client.py b/bigquery/unit_tests/test_client.py index 77c01a09aa63..e2f0618a10da 100644 --- a/bigquery/unit_tests/test_client.py +++ b/bigquery/unit_tests/test_client.py @@ -35,6 +35,7 @@ def test_ctor(self): self.assertIs(client.connection.http, http) def test_list_projects_defaults(self): + import six from google.cloud.bigquery.client import Project PROJECT_1 = 'PROJECT_ONE' PROJECT_2 = 'PROJECT_TWO' @@ -60,8 +61,8 @@ def test_list_projects_defaults(self): conn = client.connection = _Connection(DATA) iterator = client.list_projects() - iterator.update_page() - projects = list(iterator.page) + page = six.next(iterator.pages) + projects = list(page) token = iterator.next_page_token self.assertEqual(len(projects), len(DATA['projects'])) @@ -78,6 +79,8 @@ def test_list_projects_defaults(self): self.assertEqual(req['path'], '/%s' % PATH) def test_list_projects_explicit_response_missing_projects_key(self): + import six + PROJECT = 'PROJECT' PATH = 'projects' TOKEN = 'TOKEN' @@ -87,8 +90,8 @@ def test_list_projects_explicit_response_missing_projects_key(self): conn = client.connection = _Connection(DATA) iterator = client.list_projects(max_results=3, page_token=TOKEN) - iterator.update_page() - projects = list(iterator.page) + page = six.next(iterator.pages) + projects = list(page) token = iterator.next_page_token self.assertEqual(len(projects), 0) @@ -102,6 +105,7 @@ def test_list_projects_explicit_response_missing_projects_key(self): {'maxResults': 3, 'pageToken': TOKEN}) def test_list_datasets_defaults(self): + import six from google.cloud.bigquery.dataset import Dataset PROJECT = 'PROJECT' DATASET_1 = 'dataset_one' @@ -128,8 +132,8 @@ def test_list_datasets_defaults(self): conn = client.connection = _Connection(DATA) iterator = client.list_datasets() - iterator.update_page() - datasets = list(iterator.page) + page = six.next(iterator.pages) + datasets = list(page) token = iterator.next_page_token self.assertEqual(len(datasets), len(DATA['datasets'])) @@ -145,6 +149,8 @@ def test_list_datasets_defaults(self): self.assertEqual(req['path'], '/%s' % PATH) def test_list_datasets_explicit_response_missing_datasets_key(self): + import six + PROJECT = 'PROJECT' PATH = 'projects/%s/datasets' % PROJECT TOKEN = 'TOKEN' @@ -155,8 +161,8 @@ def test_list_datasets_explicit_response_missing_datasets_key(self): iterator = client.list_datasets( include_all=True, max_results=3, page_token=TOKEN) - iterator.update_page() - datasets = list(iterator.page) + page = six.next(iterator.pages) + datasets = list(page) token = iterator.next_page_token self.assertEqual(len(datasets), 0) @@ -189,6 +195,7 @@ def test_job_from_resource_unknown_type(self): client.job_from_resource({'configuration': {'nonesuch': {}}}) def test_list_jobs_defaults(self): + import six from google.cloud.bigquery.job import LoadTableFromStorageJob from google.cloud.bigquery.job import CopyJob from google.cloud.bigquery.job import ExtractTableToStorageJob @@ -301,8 +308,8 @@ def test_list_jobs_defaults(self): conn = client.connection = _Connection(DATA) iterator = client.list_jobs() - iterator.update_page() - jobs = list(iterator.page) + page = six.next(iterator.pages) + jobs = list(page) token = iterator.next_page_token self.assertEqual(len(jobs), len(DATA['jobs'])) @@ -319,6 +326,7 @@ def test_list_jobs_defaults(self): self.assertEqual(req['query_params'], {'projection': 'full'}) def test_list_jobs_load_job_wo_sourceUris(self): + import six from google.cloud.bigquery.job import LoadTableFromStorageJob PROJECT = 'PROJECT' DATASET = 'test_dataset' @@ -356,8 +364,8 @@ def test_list_jobs_load_job_wo_sourceUris(self): conn = client.connection = _Connection(DATA) iterator = client.list_jobs() - iterator.update_page() - jobs = list(iterator.page) + page = six.next(iterator.pages) + jobs = list(page) token = iterator.next_page_token self.assertEqual(len(jobs), len(DATA['jobs'])) @@ -374,6 +382,7 @@ def test_list_jobs_load_job_wo_sourceUris(self): self.assertEqual(req['query_params'], {'projection': 'full'}) def test_list_jobs_explicit_missing(self): + import six PROJECT = 'PROJECT' PATH = 'projects/%s/jobs' % PROJECT DATA = {} @@ -384,8 +393,8 @@ def test_list_jobs_explicit_missing(self): iterator = client.list_jobs(max_results=1000, page_token=TOKEN, all_users=True, state_filter='done') - iterator.update_page() - jobs = list(iterator.page) + page = six.next(iterator.pages) + jobs = list(page) token = iterator.next_page_token self.assertEqual(len(jobs), 0) diff --git a/dns/unit_tests/test_client.py b/dns/unit_tests/test_client.py index e49f43084d33..3a81e1a01754 100644 --- a/dns/unit_tests/test_client.py +++ b/dns/unit_tests/test_client.py @@ -106,6 +106,7 @@ def test_quotas_w_kind_key(self): self.assertEqual(req['path'], '/%s' % PATH) def test_list_zones_defaults(self): + import six from google.cloud.dns.zone import ManagedZone ID_1 = '123' ZONE_1 = 'zone_one' @@ -133,8 +134,8 @@ def test_list_zones_defaults(self): conn = client.connection = _Connection(DATA) iterator = client.list_zones() - iterator.update_page() - zones = list(iterator.page) + page = six.next(iterator.pages) + zones = list(page) token = iterator.next_page_token self.assertEqual(len(zones), len(DATA['managedZones'])) @@ -151,6 +152,7 @@ def test_list_zones_defaults(self): self.assertEqual(req['path'], '/%s' % PATH) def test_list_zones_explicit(self): + import six from google.cloud.dns.zone import ManagedZone ID_1 = '123' ZONE_1 = 'zone_one' @@ -177,8 +179,8 @@ def test_list_zones_explicit(self): conn = client.connection = _Connection(DATA) iterator = client.list_zones(max_results=3, page_token=TOKEN) - iterator.update_page() - zones = list(iterator.page) + page = six.next(iterator.pages) + zones = list(page) token = iterator.next_page_token self.assertEqual(len(zones), len(DATA['managedZones'])) diff --git a/dns/unit_tests/test_zone.py b/dns/unit_tests/test_zone.py index 3616cc438e8a..63b34339bc7e 100644 --- a/dns/unit_tests/test_zone.py +++ b/dns/unit_tests/test_zone.py @@ -409,6 +409,7 @@ def test_delete_w_alternate_client(self): self.assertEqual(req['path'], '/%s' % PATH) def test_list_resource_record_sets_defaults(self): + import six from google.cloud.dns.resource_record_set import ResourceRecordSet PATH = 'projects/%s/managedZones/%s/rrsets' % ( self.PROJECT, self.ZONE_NAME) @@ -442,8 +443,8 @@ def test_list_resource_record_sets_defaults(self): iterator = zone.list_resource_record_sets() self.assertIs(zone, iterator.zone) - iterator.update_page() - rrsets = list(iterator.page) + page = six.next(iterator.pages) + rrsets = list(page) token = iterator.next_page_token self.assertEqual(len(rrsets), len(DATA['rrsets'])) @@ -461,6 +462,7 @@ def test_list_resource_record_sets_defaults(self): self.assertEqual(req['path'], '/%s' % PATH) def test_list_resource_record_sets_explicit(self): + import six from google.cloud.dns.resource_record_set import ResourceRecordSet PATH = 'projects/%s/managedZones/%s/rrsets' % ( self.PROJECT, self.ZONE_NAME) @@ -496,8 +498,8 @@ def test_list_resource_record_sets_explicit(self): iterator = zone.list_resource_record_sets( max_results=3, page_token=TOKEN, client=client2) self.assertIs(zone, iterator.zone) - iterator.update_page() - rrsets = list(iterator.page) + page = six.next(iterator.pages) + rrsets = list(page) token = iterator.next_page_token self.assertEqual(len(rrsets), len(DATA['rrsets'])) @@ -553,6 +555,7 @@ def _get_changes(self, token, changes_name): return result def test_list_changes_defaults(self): + import six from google.cloud.dns.changes import Changes from google.cloud.dns.resource_record_set import ResourceRecordSet @@ -569,8 +572,8 @@ def test_list_changes_defaults(self): iterator = zone.list_changes() self.assertIs(zone, iterator.zone) - iterator.update_page() - changes = list(iterator.page) + page = six.next(iterator.pages) + changes = list(page) token = iterator.next_page_token self.assertEqual(len(changes), len(data['changes'])) @@ -606,6 +609,7 @@ def test_list_changes_defaults(self): self.assertEqual(req['path'], '/%s' % (path,)) def test_list_changes_explicit(self): + import six from google.cloud.dns.changes import Changes from google.cloud.dns.resource_record_set import ResourceRecordSet @@ -624,8 +628,8 @@ def test_list_changes_explicit(self): iterator = zone.list_changes( max_results=3, page_token=page_token, client=client2) self.assertIs(zone, iterator.zone) - iterator.update_page() - changes = list(iterator.page) + page = six.next(iterator.pages) + changes = list(page) token = iterator.next_page_token self.assertEqual(len(changes), len(data['changes'])) diff --git a/resource_manager/unit_tests/test_client.py b/resource_manager/unit_tests/test_client.py index 724b48e26e44..cdc4159dd8ab 100644 --- a/resource_manager/unit_tests/test_client.py +++ b/resource_manager/unit_tests/test_client.py @@ -229,6 +229,7 @@ def test_page_empty_response(self): self.assertEqual(list(page), []) def test_page_non_empty_response(self): + import six from google.cloud.resource_manager.project import Project project_id = 'project-id' @@ -253,10 +254,9 @@ def dummy_response(): iterator = client.list_projects() iterator._get_next_page_response = dummy_response - iterator.update_page() - page = iterator.page + page = six.next(iterator.pages) self.assertEqual(page.num_items, 1) - project = iterator.next() + project = six.next(page) self.assertEqual(page.remaining, 0) self.assertIsInstance(project, Project) self.assertEqual(project.project_id, project_id) diff --git a/storage/unit_tests/test_bucket.py b/storage/unit_tests/test_bucket.py index 59f48dedbfd8..263a09e9df09 100644 --- a/storage/unit_tests/test_bucket.py +++ b/storage/unit_tests/test_bucket.py @@ -991,6 +991,7 @@ def test_page_empty_response(self): self.assertEqual(iterator.prefixes, set()) def test_page_non_empty_response(self): + import six from google.cloud.storage.blob import Blob blob_name = 'blob-name' @@ -1006,17 +1007,17 @@ def dummy_response(): iterator = bucket.list_blobs() iterator._get_next_page_response = dummy_response - iterator.update_page() - page = iterator.page + page = six.next(iterator.pages) self.assertEqual(page.prefixes, ('foo',)) self.assertEqual(page.num_items, 1) - blob = iterator.next() + blob = six.next(page) self.assertEqual(page.remaining, 0) self.assertIsInstance(blob, Blob) self.assertEqual(blob.name, blob_name) self.assertEqual(iterator.prefixes, set(['foo'])) def test_cumulative_prefixes(self): + import six from google.cloud.storage.blob import Blob BLOB_NAME = 'blob-name1' @@ -1041,18 +1042,17 @@ def dummy_response(): iterator._get_next_page_response = dummy_response # Parse first response. - iterator.update_page() - page1 = iterator.page + pages_iter = iterator.pages + page1 = six.next(pages_iter) self.assertEqual(page1.prefixes, ('foo',)) self.assertEqual(page1.num_items, 1) - blob = iterator.next() + blob = six.next(page1) self.assertEqual(page1.remaining, 0) self.assertIsInstance(blob, Blob) self.assertEqual(blob.name, BLOB_NAME) self.assertEqual(iterator.prefixes, set(['foo'])) # Parse second response. - iterator.update_page() - page2 = iterator.page + page2 = six.next(pages_iter) self.assertEqual(page2.prefixes, ('bar',)) self.assertEqual(page2.num_items, 0) self.assertEqual(iterator.prefixes, set(['foo', 'bar'])) diff --git a/storage/unit_tests/test_client.py b/storage/unit_tests/test_client.py index 5cd6e2b1694e..2b55fff71968 100644 --- a/storage/unit_tests/test_client.py +++ b/storage/unit_tests/test_client.py @@ -381,6 +381,7 @@ def test_page_empty_response(self): self.assertEqual(list(page), []) def test_page_non_empty_response(self): + import six from google.cloud.storage.bucket import Bucket project = 'PROJECT' @@ -396,10 +397,9 @@ def dummy_response(): iterator = client.list_buckets() iterator._get_next_page_response = dummy_response - iterator.update_page() - page = iterator.page + page = six.next(iterator.pages) self.assertEqual(page.num_items, 1) - bucket = iterator.next() + bucket = six.next(page) self.assertEqual(page.remaining, 0) self.assertIsInstance(bucket, Bucket) self.assertEqual(bucket.name, blob_name) From 9a504c665f696cf81d7ad657aa904a2abebd242b Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Mon, 24 Oct 2016 00:25:29 -0700 Subject: [PATCH 8/8] Fixing use of Iterator in storage system tests. --- system_tests/storage.py | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/system_tests/storage.py b/system_tests/storage.py index d595f54e70d1..c05d517420e6 100644 --- a/system_tests/storage.py +++ b/system_tests/storage.py @@ -257,13 +257,20 @@ def test_paginate_files(self): truncation_size = 1 count = len(self.FILENAMES) - truncation_size iterator = self.bucket.list_blobs(max_results=count) - iterator.update_page() - blobs = list(iterator.page) + page_iter = iterator.pages + + page1 = six.next(page_iter) + blobs = list(page1) self.assertEqual(len(blobs), count) self.assertIsNotNone(iterator.next_page_token) - - iterator.update_page() - last_blobs = list(iterator.page) + # Technically the iterator is exhausted. + self.assertEqual(iterator.num_results, iterator.max_results) + # But we modify the iterator to continue paging after + # articially stopping after ``count`` items. + iterator.max_results = None + + page2 = six.next(page_iter) + last_blobs = list(page2) self.assertEqual(len(last_blobs), truncation_size) @@ -301,8 +308,8 @@ def tearDownClass(cls): @RetryErrors(unittest.TestCase.failureException) def test_root_level_w_delimiter(self): iterator = self.bucket.list_blobs(delimiter='/') - iterator.update_page() - blobs = list(iterator.page) + page = six.next(iterator.pages) + blobs = list(page) self.assertEqual([blob.name for blob in blobs], ['file01.txt']) self.assertIsNone(iterator.next_page_token) self.assertEqual(iterator.prefixes, set(['parent/'])) @@ -310,8 +317,8 @@ def test_root_level_w_delimiter(self): @RetryErrors(unittest.TestCase.failureException) def test_first_level(self): iterator = self.bucket.list_blobs(delimiter='/', prefix='parent/') - iterator.update_page() - blobs = list(iterator.page) + page = six.next(iterator.pages) + blobs = list(page) self.assertEqual([blob.name for blob in blobs], ['parent/file11.txt']) self.assertIsNone(iterator.next_page_token) self.assertEqual(iterator.prefixes, set(['parent/child/'])) @@ -325,8 +332,8 @@ def test_second_level(self): iterator = self.bucket.list_blobs(delimiter='/', prefix='parent/child/') - iterator.update_page() - blobs = list(iterator.page) + page = six.next(iterator.pages) + blobs = list(page) self.assertEqual([blob.name for blob in blobs], expected_names) self.assertIsNone(iterator.next_page_token) @@ -341,8 +348,8 @@ def test_third_level(self): # Exercise a layer deeper to illustrate this. iterator = self.bucket.list_blobs(delimiter='/', prefix='parent/child/grand/') - iterator.update_page() - blobs = list(iterator.page) + page = six.next(iterator.pages) + blobs = list(page) self.assertEqual([blob.name for blob in blobs], ['parent/child/grand/file31.txt']) self.assertIsNone(iterator.next_page_token)