Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Making max_results part of the base Iterator class. #2490

Merged
merged 1 commit into from
Oct 4, 2016
Merged

Making max_results part of the base Iterator class. #2490

merged 1 commit into from
Oct 4, 2016

Conversation

dhermes
Copy link
Contributor

@dhermes dhermes commented Oct 4, 2016

In the process, also making sure to lower maxResults on subsequent requests.

Fixes #1467.

@dhermes dhermes added api: storage Issues related to the Cloud Storage API. api: core labels Oct 4, 2016
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Oct 4, 2016
dhermes
dhermes commented Oct 4, 2016
View reviewed changes
core/google/cloud/iterator.py
except StopIteration:
item = self._update_items()
self.num_results += 1
return item

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@dhermes
Copy link
Contributor Author

dhermes commented Oct 4, 2016

I'm very open to suggestions on other unit / system tests I could add to this PR. Just wanted to get it out there for discussion's sake.

@dhermes
Making max_results part of the base Iterator class.
78b09c5 
In the process, also making sure to lower maxResults
on subsequent requests.

Fixes googleapis#1467.
@tseaver
Copy link
Contributor

tseaver commented Oct 4, 2016

I would've thought changing to use google.cloud.iterator.MethodIterator everywhere would have been more straightforward (and giving us a clearer path toward a general solution for #895). The derived iterator classes feel like roach motels to me.

@dhermes
Copy link
Contributor Author

dhermes commented Oct 4, 2016

@tseaver This is just a band-aid for #1467. I will probably start from scratch / build on top of MethodIterator when finishing #895.

@tseaver tseaver mentioned this pull request Oct 4, 2016
Closed
@dhermes dhermes merged commit 8c50e41 into googleapis:master Oct 4, 2016
@dhermes dhermes deleted the fix-1467 branch October 4, 2016 21:55
@dhermes dhermes mentioned this pull request Nov 14, 2016
Merged
richkadel pushed a commit to richkadel/google-cloud-python that referenced this pull request May 6, 2017
@dhermes
Merge pull request googleapis#2490 from dhermes/fix-1467
4a82b09 
Making max_results part of the base Iterator class.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@daspecster daspecster daspecster left review comments

@tseaver tseaver tseaver approved these changes

Assignees

@tseaver tseaver

Labels
api: core api: storage Issues related to the Cloud Storage API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dhermes @tseaver @daspecster @googlebot