Re-architect Iterator class. by dhermes · Pull Request #2531 · googleapis/google-cloud-python · GitHub

This PR should definitely reviewed commit-by-commit.

Primary changes are:

Have an iterable Page base-class for iterating over a single page of a result set
Move most custom iterator logic back up into the base page and iterator classes

Intended to hold and slice up state that has already been retrieved from the server.

This is so that the owned items iterator management could be done on the base class and the child classes just need to worry about converting the JSON values to whatever type is required.

This state can never happen since a StopIteration will occur before the method would ever be called without a token.

Also doing a tiny re-org in constructor to separate the attributes which change and those which don't.

Making JSON API call helpers non-public. This reduces the interface to methods relevant to iterating.

My one concern is how a service like datastore will work.

This abstraction seems to completely hides cursors, but cursors often need to be passed between client and server in datastore (think client-side pagination). I'm wondering how that would fit in here?

My one concern is how a service like datastore will work.

Datastore has it's own specialized Iterator. For the time being, the google.cloud.iterator.Iterator class is for JSON-based APIs that define a nextPageToken in response pages.

Though a "next step" would be to make the iterator support gRPC / GAPIC generated classes, so we may be able to tackle datastore there. (Just not in this PR.)

Sorry I haven't responded on this @dhermes. I don't have much feedback though.

You actually just answered the question I did have.

Though a "next step" would be to make the iterator support gRPC / GAPIC generated classes, so we may be able to tackle datastore there. (Just not in this PR.)

LGTM

This is to lower the burden on implementers. The previous approach (requiring a Page and Iterator subclass) ended up causing lots of copy-pasta docstrings that were just a distraction. Follow up to googleapis#2531.

Re-architect Iterator class.

This is to lower the burden on implementers. The previous approach (requiring a Page and Iterator subclass) ended up causing lots of copy-pasta docstrings that were just a distraction. Follow up to googleapis#2531.

This is to lower the burden on implementers. The previous approach (requiring a Page and Iterator subclass) ended up causing lots of copy-pasta docstrings that were just a distraction. Follow up to #2531.

Re-architect Iterator class.

This is to lower the burden on implementers. The previous approach (requiring a Page and Iterator subclass) ended up causing lots of copy-pasta docstrings that were just a distraction. Follow up to #2531.

dhermes added api: storage Issues related to the Cloud Storage API. api: core api: cloudresourcemanager Issues related to the Resource Manager API. labels Oct 12, 2016

dhermes assigned daspecster and tseaver Oct 12, 2016

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Oct 12, 2016

dhermes added 13 commits October 12, 2016 01:05

Implementing basic Page class to help with Iterator.

6636714

Intended to hold and slice up state that has already been retrieved from the server.

Collapsing get_items_from_response behavior into Page subclasses.

e9d600a

Collapsing Page __init__ functionality into base class.

819e644

Replacing get_items_from_response() with a page class.

afc0662

Making path an optional argument to Iterator.

ff85b24

Swapping iterator's _next_item() for _item_to_value().

5326c88

This is so that the owned items iterator management could be done on the base class and the child classes just need to worry about converting the JSON values to whatever type is required.

Exposing the current page in iterator.

f4c2bcd

Removing RuntimeError from iterator's get_next_page_response().

4988a2a

This state can never happen since a StopIteration will occur before the method would ever be called without a token.

Adding a page reset to iterator's reset.

736c3a0

Also doing a tiny re-org in constructor to separate the attributes which change and those which don't.

Updating storage system test after Iterator refactor.

122ec89

Lint cleanup: unused imports.

c6218a2

Removing unused MethodIterator.

c92961d

Implementation hiding in Iterator class.

d9824c6

Making JSON API call helpers non-public. This reduces the interface to methods relevant to iterating.

dhermes force-pushed the revamp-iterator branch from 6d50cf6 to d9824c6 Compare October 12, 2016 08:07

dhermes mentioned this pull request Oct 12, 2016

Moving to iterators in DNS. #2532

Closed

theacodes reviewed Oct 14, 2016

View reviewed changes

dhermes merged commit 9a5ddd5 into googleapis:master Oct 14, 2016

dhermes deleted the revamp-iterator branch October 14, 2016 19:09

dhermes mentioned this pull request Oct 14, 2016

Moving backend specific behavior from Page to Iterator. #2545

Merged

dhermes mentioned this pull request Nov 14, 2016

Upgrading core to version to 0.21.0. #2733

Merged

dhermes mentioned this pull request Dec 9, 2016

Update versions for mega-release. #2846

Merged

tseaver mentioned this pull request Feb 6, 2017

Update iterator to take a method and **kwargs #1554

Closed

richkadel pushed a commit to richkadel/google-cloud-python that referenced this pull request May 6, 2017

Merge pull request googleapis#2531 from dhermes/revamp-iterator

ba13299

Re-architect Iterator class.

theacodes unassigned daspecster Sep 28, 2018

parthea pushed a commit that referenced this pull request Nov 24, 2025

Merge pull request #2531 from dhermes/revamp-iterator

3c879aa

Re-architect Iterator class.

ohmayr added a commit that referenced this pull request Mar 13, 2026

chore: cache proto context to optimize generator performance (#2531 )

35d1b0f

Re-architect Iterator class. by dhermes · Pull Request #2531 · googleapis/google-cloud-python · GitHub

Uh oh!

Re-architect Iterator class. - #2531

Re-architect Iterator class.#2531
dhermes merged 13 commits into
googleapis:masterfrom
dhermes:revamp-iterator

dhermes commented Oct 12, 2016

Uh oh!

theacodes left a comment •
edited
Loading

Uh oh!

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

dhermes commented Oct 14, 2016

Uh oh!

daspecster commented Oct 14, 2016

Uh oh!

theacodes commented Oct 14, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

+                      """The :class:`Page` is an iterator."""
+                      return self
+                  def _item_to_value(self, item):

+                      return self._num_items
+                  @property
+                  def remaining(self):

Re-architect Iterator class. by dhermes · Pull Request #2531 · googleapis/google-cloud-python · GitHub

Uh oh!

Re-architect Iterator class. - #2531

Re-architect Iterator class.#2531dhermes merged 13 commits intogoogleapis:mastergoogleapis/google-cloud-python:masterfrom dhermes:revamp-iteratordhermes/google-cloud-python:revamp-iteratorCopy head branch name to clipboard

Conversation

dhermes commented Oct 12, 2016

Uh oh!

theacodes left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

dhermes commented Oct 14, 2016

Uh oh!

daspecster commented Oct 14, 2016

Uh oh!

theacodes commented Oct 14, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Re-architect Iterator class.#2531
dhermes merged 13 commits into
googleapis:masterfrom
dhermes:revamp-iterator

theacodes left a comment •
edited
Loading