FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Removing Iterator and Page subclasses. by dhermes · Pull Request #2558 · googleapis/google-cloud-python · GitHub

/ google-cloud-python Public

Removing Iterator and Page subclasses. - #2558

Merged
dhermes merged 2 commits into
googleapis:masterfrom
dhermes:no-more-iterator-subclasses
Oct 18, 2016
ConversationCommits (2)ChecksFiles changed
Merged

Removing Iterator and Page subclasses.#2558
dhermes merged 2 commits into
googleapis:masterfrom
dhermes:no-more-iterator-subclasses

Conversation

dhermes commented Oct 18, 2016

Copy link
Copy Markdown
Contributor

Instead require Iterator takes:

  • a well-formed path for the request
  • a callable to convert a JSON item to native obj.
  • (optional) the key in a response holding all items
  • (optional) a page_start (acts as proxy for Page.__init__)

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Oct 18, 2016
dhermes mentioned this pull request Oct 18, 2016
Closed

theacodes commented Oct 18, 2016

Copy link
Copy Markdown
Contributor

@dhermes the general idea here LGTM.

dhermes commented Oct 18, 2016

Copy link
Copy Markdown
Contributor Author

@tseaver PTAL. I'd really like to get the list_foo() migration done

Removing Iterator and Page subclasses.
8cc9341 
Instead require `Iterator` takes:
- a well-formed path for the request
- a callable to convert a JSON item to native obj.
- (optional) the key in a response holding all items
- (optional) a `page_start` (acts as proxy for `Page.__init__`)
dhermes force-pushed the no-more-iterator-subclasses branch from db650b7 to 8cc9341 Compare October 18, 2016 16:39
This was referenced Oct 18, 2016
Merged
Merged

daspecster commented Oct 18, 2016

Copy link
Copy Markdown
Contributor

Ditto @jonparrott.

tseaver reviewed Oct 18, 2016
View reviewed changes

tseaver left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Except for the odd bit of having item_to_value be optional, this looks right to me.

Comment thread core/google/cloud/iterator.py
:param item_to_value: (Optional) Callable to convert an item from JSON
into the native object. Assumed signature
takes an :class:`Iterator` and a dictionary
holding a single 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

Comment thread core/google/cloud/iterator.py Outdated

:type items_key: str
:param items_key: The key used to grab retrieved items from an API
response. Defaults to :data:`DEFAULT_ITEMS_KEY`.

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 commented Oct 18, 2016

Copy link
Copy Markdown
Contributor Author

@tseaver Anything else?

dhermes commented Oct 18, 2016 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@tseaver So I'll add "(Optional)" to the docstring and make item_to_value a positional / required argument. (UPDATE: And fix the lint error. Wrong PR.)

Good to merge after those changes are in?

tseaver commented Oct 18, 2016

Copy link
Copy Markdown
Contributor

@dhermes

Good to merge after those changes are in?

Yup.

Making item_to_value a required argument.
b6f5328 
Also adding "(Optional)" to items_key docstring.

dhermes commented Oct 18, 2016

Copy link
Copy Markdown
Contributor Author

OK, waited an hour for Travis. Merging. (There is a lot to do after.)

dhermes merged commit e575f81 into googleapis:master Oct 18, 2016
dhermes deleted the no-more-iterator-subclasses branch October 18, 2016 20:21
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
Merge pull request googleapis#2558 from dhermes/no-more-iterator-subc…
cb5bb26 
…lasses

Removing Iterator and Page subclasses.
parthea pushed a commit that referenced this pull request Jun 4, 2023
Merge pull request #2558 from dhermes/no-more-iterator-subclasses
a001cc1 
Removing Iterator and Page subclasses.
parthea pushed a commit that referenced this pull request Oct 21, 2023
fix: Use different versions of pytest for python 2 and python3 [(#2558)…
f86f985 
…](GoogleCloudPlatform/python-docs-samples#2558)

* fix: Use different versions of pytest for python 2 and python3

* fix: delete extra pytest dep

* fix: update pytest dependencies in requirements.txt
parthea pushed a commit that referenced this pull request Nov 24, 2025
Merge pull request #2558 from dhermes/no-more-iterator-subclasses
86f60f7 
Removing Iterator and Page subclasses.
parthea pushed a commit that referenced this pull request Mar 9, 2026
Merge pull request #2558 from dhermes/no-more-iterator-subclasses
21686ca 
Removing Iterator and Page subclasses.
ohmayr added a commit that referenced this pull request Mar 13, 2026
chore: fix service yaml command in nox sessions (#2558)
2e334b3
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
tseaver tseaver left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

tseaver

theacodes

Labels

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.

5 participants

Back | FazBrowse Home | New Git URL