-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement Iterator.pages and simplify items iteration #2594
Conversation
11a4f48
to
0ff8ece
Compare
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).
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.
0ff8ece
to
9a504c6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Big improvement in the API surface for iterators.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice.
|
||
:rtype: :class:`Iterator` | ||
:returns: Current instance. | ||
:rtype: :class:`~types.GeneratorType` |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Broken by simultaneous merges of googleapis#2594 and googleapis#2485.
Implement Iterator.pages and simplify items iteration
Broken by simultaneous merges of googleapis#2594 and googleapis#2485.
Fixes #2548. As mentioned in #2529, this came from trying to design for the case that a page of responses came from GAX instead of from an HTTP response.
I have left alone
Iterator.next_page_token
andIterator.page_number
for now but think the token should be moved to thePage
and the page number should just be removed (people can just useenumerate(iterator.pages)
if they want the page number).NOTE: Has #2592 as diffbase.