From b59285f32b3868c7fdfecd92023882afe5e09ce0 Mon Sep 17 00:00:00 2001 From: Dov Shlachter Date: Wed, 16 Oct 2019 14:06:08 -0700 Subject: [PATCH 1/8] Provide a 'raw_page' field for page_iterator.Page Some paginated response messages include additional fields that users may wish to inspect. --- api_core/google/api_core/page_iterator.py | 16 +++++++++---- api_core/tests/unit/test_page_iterator.py | 28 ++++++++++++++++++----- 2 files changed, 34 insertions(+), 10 deletions(-) diff --git a/api_core/google/api_core/page_iterator.py b/api_core/google/api_core/page_iterator.py index 3ac5904399b0..ade9185d5cd9 100644 --- a/api_core/google/api_core/page_iterator.py +++ b/api_core/google/api_core/page_iterator.py @@ -96,14 +96,22 @@ class Page(object): Callable to convert an item from the type in the raw API response into the native object. Will be called with the iterator and a single item. + raw_page Optional[google.protobuf.message.Message]: + The raw page response. """ - def __init__(self, parent, items, item_to_value): + def __init__(self, parent, items, item_to_value, raw_page): self._parent = parent self._num_items = len(items) self._remaining = self._num_items self._item_iter = iter(items) self._item_to_value = item_to_value + self._raw_page = raw_page + + @property + def raw_page(self): + """google.protobuf.message.Message""" + return self._raw_page @property def num_items(self): @@ -360,7 +368,7 @@ def _next_page(self): if self._has_next_page(): response = self._get_next_page_response() items = response.get(self._items_key, ()) - page = Page(self, items, self.item_to_value) + page = Page(self, items, self.item_to_value, response) self._page_start(self, page, response) self.next_page_token = response.get(self._next_token) return page @@ -454,7 +462,7 @@ def _next_page(self): """ try: items = six.next(self._gax_page_iter) - page = Page(self, items, self.item_to_value) + page = Page(self, items, self.item_to_value, None) self.next_page_token = self._gax_page_iter.page_token or None return page except StopIteration: @@ -527,7 +535,7 @@ def _next_page(self): self.next_page_token = getattr(response, self._response_token_field) items = getattr(response, self._items_field) - page = Page(self, items, self.item_to_value) + page = Page(self, items, self.item_to_value, response) return page diff --git a/api_core/tests/unit/test_page_iterator.py b/api_core/tests/unit/test_page_iterator.py index 6335001bcf41..7b2367ebbf9c 100644 --- a/api_core/tests/unit/test_page_iterator.py +++ b/api_core/tests/unit/test_page_iterator.py @@ -30,7 +30,7 @@ def test_constructor(self): parent = mock.sentinel.parent item_to_value = mock.sentinel.item_to_value - page = page_iterator.Page(parent, (1, 2, 3), item_to_value) + page = page_iterator.Page(parent, (1, 2, 3), item_to_value, None) assert page.num_items == 3 assert page.remaining == 3 @@ -38,7 +38,7 @@ def test_constructor(self): assert page._item_to_value is item_to_value def test___iter__(self): - page = page_iterator.Page(None, (), None) + page = page_iterator.Page(None, (), None, None) assert iter(page) is page def test_iterator_calls_parent_item_to_value(self): @@ -48,7 +48,7 @@ def test_iterator_calls_parent_item_to_value(self): side_effect=lambda iterator, value: value, spec=["__call__"] ) - page = page_iterator.Page(parent, (10, 11, 12), item_to_value) + page = page_iterator.Page(parent, (10, 11, 12), item_to_value, None) page._remaining = 100 assert item_to_value.call_count == 0 @@ -69,6 +69,22 @@ def test_iterator_calls_parent_item_to_value(self): item_to_value.assert_called_with(parent, 12) assert page.remaining == 97 + def test_raw_page(self): + parent = mock.sentinel.parent + item_to_value = mock.sentinel.item_to_value + + raw_page = mock.sentinel.raw_page + + page = page_iterator.Page(parent, (1, 2, 3), item_to_value, raw_page) + assert page.raw_page is raw_page + + try: + page.raw_page = None + except AttributeError: + pass + else: + assert False, "The raw_page attribute should be a read-only property" + class PageIteratorImpl(page_iterator.Iterator): def _next_page(self): @@ -116,7 +132,7 @@ def test_pages_property_restart(self): def test__page_iter_increment(self): iterator = PageIteratorImpl(None, None) page = page_iterator.Page( - iterator, ("item",), page_iterator._item_to_value_identity + iterator, ("item",), page_iterator._item_to_value_identity, None ) iterator._next_page = mock.Mock(side_effect=[page, None]) @@ -147,10 +163,10 @@ def test__items_iter(self): # Make pages from mock responses parent = mock.sentinel.parent page1 = page_iterator.Page( - parent, (item1, item2), page_iterator._item_to_value_identity + parent, (item1, item2), page_iterator._item_to_value_identity, None ) page2 = page_iterator.Page( - parent, (item3,), page_iterator._item_to_value_identity + parent, (item3,), page_iterator._item_to_value_identity, None ) iterator = PageIteratorImpl(None, None) From b30992e41250d53530f76e2cc60ae59d901f219c Mon Sep 17 00:00:00 2001 From: Dov Shlachter Date: Wed, 16 Oct 2019 14:39:09 -0700 Subject: [PATCH 2/8] Ignore 'assert False' in coverage --- api_core/.coveragerc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/api_core/.coveragerc b/api_core/.coveragerc index d097511c3124..ed0cb9c44c61 100644 --- a/api_core/.coveragerc +++ b/api_core/.coveragerc @@ -11,3 +11,5 @@ exclude_lines = def __repr__ # Ignore abstract methods raise NotImplementedError + # Ignore assert False, meant to prevent false positive tests + assert False \ No newline at end of file From 867ecf4996eccf9119a0026aaafe77abf261b84e Mon Sep 17 00:00:00 2001 From: Dov Shlachter Date: Fri, 25 Oct 2019 11:13:12 -0700 Subject: [PATCH 3/8] Add trailing newline to api_core/.coveragerc --- api_core/.coveragerc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/api_core/.coveragerc b/api_core/.coveragerc index ed0cb9c44c61..b38eb25b3e17 100644 --- a/api_core/.coveragerc +++ b/api_core/.coveragerc @@ -12,4 +12,5 @@ exclude_lines = # Ignore abstract methods raise NotImplementedError # Ignore assert False, meant to prevent false positive tests - assert False \ No newline at end of file + assert False + \ No newline at end of file From aac9fcc8159155960f4c0f3e0e3dbc2814252b64 Mon Sep 17 00:00:00 2001 From: Dov Shlachter Date: Fri, 25 Oct 2019 11:38:00 -0700 Subject: [PATCH 4/8] Whitespace fix --- api_core/.coveragerc | 1 - 1 file changed, 1 deletion(-) diff --git a/api_core/.coveragerc b/api_core/.coveragerc index b38eb25b3e17..b39306836ab9 100644 --- a/api_core/.coveragerc +++ b/api_core/.coveragerc @@ -13,4 +13,3 @@ exclude_lines = raise NotImplementedError # Ignore assert False, meant to prevent false positive tests assert False - \ No newline at end of file From 10c95b2cf602dcca4c84f864decba7e04daf46b2 Mon Sep 17 00:00:00 2001 From: Dov Shlachter Date: Wed, 30 Oct 2019 10:55:11 -0700 Subject: [PATCH 5/8] Integrate reviews --- api_core/google/api_core/page_iterator.py | 8 ++++---- api_core/tests/unit/test_page_iterator.py | 22 ++++++++-------------- 2 files changed, 12 insertions(+), 18 deletions(-) diff --git a/api_core/google/api_core/page_iterator.py b/api_core/google/api_core/page_iterator.py index ade9185d5cd9..11a92d38f3ce 100644 --- a/api_core/google/api_core/page_iterator.py +++ b/api_core/google/api_core/page_iterator.py @@ -100,7 +100,7 @@ class Page(object): The raw page response. """ - def __init__(self, parent, items, item_to_value, raw_page): + def __init__(self, parent, items, item_to_value, raw_page=None): self._parent = parent self._num_items = len(items) self._remaining = self._num_items @@ -368,7 +368,7 @@ def _next_page(self): if self._has_next_page(): response = self._get_next_page_response() items = response.get(self._items_key, ()) - page = Page(self, items, self.item_to_value, response) + page = Page(self, items, self.item_to_value, raw_page=response) self._page_start(self, page, response) self.next_page_token = response.get(self._next_token) return page @@ -462,7 +462,7 @@ def _next_page(self): """ try: items = six.next(self._gax_page_iter) - page = Page(self, items, self.item_to_value, None) + page = Page(self, items, self.item_to_value) self.next_page_token = self._gax_page_iter.page_token or None return page except StopIteration: @@ -535,7 +535,7 @@ def _next_page(self): self.next_page_token = getattr(response, self._response_token_field) items = getattr(response, self._items_field) - page = Page(self, items, self.item_to_value, response) + page = Page(self, items, self.item_to_value, raw_page=response) return page diff --git a/api_core/tests/unit/test_page_iterator.py b/api_core/tests/unit/test_page_iterator.py index 7b2367ebbf9c..99eca19b7cbe 100644 --- a/api_core/tests/unit/test_page_iterator.py +++ b/api_core/tests/unit/test_page_iterator.py @@ -30,12 +30,13 @@ def test_constructor(self): parent = mock.sentinel.parent item_to_value = mock.sentinel.item_to_value - page = page_iterator.Page(parent, (1, 2, 3), item_to_value, None) + page = page_iterator.Page(parent, (1, 2, 3), item_to_value) assert page.num_items == 3 assert page.remaining == 3 assert page._parent is parent assert page._item_to_value is item_to_value + assert page.raw_page is None def test___iter__(self): page = page_iterator.Page(None, (), None, None) @@ -48,7 +49,7 @@ def test_iterator_calls_parent_item_to_value(self): side_effect=lambda iterator, value: value, spec=["__call__"] ) - page = page_iterator.Page(parent, (10, 11, 12), item_to_value, None) + page = page_iterator.Page(parent, (10, 11, 12), item_to_value) page._remaining = 100 assert item_to_value.call_count == 0 @@ -75,15 +76,11 @@ def test_raw_page(self): raw_page = mock.sentinel.raw_page - page = page_iterator.Page(parent, (1, 2, 3), item_to_value, raw_page) + page = page_iterator.Page(parent, (1, 2, 3), item_to_value, raw_page=raw_page) assert page.raw_page is raw_page - try: + self.assertRaises(AttributeError): page.raw_page = None - except AttributeError: - pass - else: - assert False, "The raw_page attribute should be a read-only property" class PageIteratorImpl(page_iterator.Iterator): @@ -132,8 +129,7 @@ def test_pages_property_restart(self): def test__page_iter_increment(self): iterator = PageIteratorImpl(None, None) page = page_iterator.Page( - iterator, ("item",), page_iterator._item_to_value_identity, None - ) + iterator, ("item",), page_iterator._item_to_value_identity) iterator._next_page = mock.Mock(side_effect=[page, None]) assert iterator.num_results == 0 @@ -163,11 +159,9 @@ def test__items_iter(self): # Make pages from mock responses parent = mock.sentinel.parent page1 = page_iterator.Page( - parent, (item1, item2), page_iterator._item_to_value_identity, None - ) + parent, (item1, item2), page_iterator._item_to_value_identity) page2 = page_iterator.Page( - parent, (item3,), page_iterator._item_to_value_identity, None - ) + parent, (item3,), page_iterator._item_to_value_identity) iterator = PageIteratorImpl(None, None) iterator._next_page = mock.Mock(side_effect=[page1, page2, None]) From 40f62ea86804382d0b74fa9ea7df274667b917d1 Mon Sep 17 00:00:00 2001 From: Dov Shlachter Date: Thu, 31 Oct 2019 13:33:49 -0700 Subject: [PATCH 6/8] Fix a boneheaded error --- api_core/tests/unit/test_page_iterator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api_core/tests/unit/test_page_iterator.py b/api_core/tests/unit/test_page_iterator.py index 99eca19b7cbe..debcf14ab0e2 100644 --- a/api_core/tests/unit/test_page_iterator.py +++ b/api_core/tests/unit/test_page_iterator.py @@ -79,7 +79,7 @@ def test_raw_page(self): page = page_iterator.Page(parent, (1, 2, 3), item_to_value, raw_page=raw_page) assert page.raw_page is raw_page - self.assertRaises(AttributeError): + with self.assertRaises(AttributeError): page.raw_page = None From f6bc128f61a21041c5289f6d52345f0ce745afcb Mon Sep 17 00:00:00 2001 From: Dov Shlachter Date: Fri, 1 Nov 2019 09:30:23 -0700 Subject: [PATCH 7/8] Real fix, can now run tests --- api_core/tests/unit/test_page_iterator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api_core/tests/unit/test_page_iterator.py b/api_core/tests/unit/test_page_iterator.py index debcf14ab0e2..2bf742492889 100644 --- a/api_core/tests/unit/test_page_iterator.py +++ b/api_core/tests/unit/test_page_iterator.py @@ -79,7 +79,7 @@ def test_raw_page(self): page = page_iterator.Page(parent, (1, 2, 3), item_to_value, raw_page=raw_page) assert page.raw_page is raw_page - with self.assertRaises(AttributeError): + with pytest.raises(AttributeError): page.raw_page = None From 14e70aaffc786e80f0b76592fada2117bad5da40 Mon Sep 17 00:00:00 2001 From: Dov Shlachter Date: Fri, 1 Nov 2019 09:35:49 -0700 Subject: [PATCH 8/8] Remove an assert False --- api_core/.coveragerc | 2 -- 1 file changed, 2 deletions(-) diff --git a/api_core/.coveragerc b/api_core/.coveragerc index b39306836ab9..d097511c3124 100644 --- a/api_core/.coveragerc +++ b/api_core/.coveragerc @@ -11,5 +11,3 @@ exclude_lines = def __repr__ # Ignore abstract methods raise NotImplementedError - # Ignore assert False, meant to prevent false positive tests - assert False