Visitar URL original
Migrate datastore iterator to use base iterator class by dhermes · Pull Request #2706 · googleapis/google-cloud-python · GitHub
Skip to content

Migrate datastore iterator to use base iterator class - #2706

Merged
dhermes merged 13 commits into
googleapis:masterfrom
dhermes:migrate-datastore-iterator
Nov 9, 2016
Merged

Migrate datastore iterator to use base iterator class#2706
dhermes merged 13 commits into
googleapis:masterfrom
dhermes:migrate-datastore-iterator

Conversation

@dhermes

@dhermes dhermes commented Nov 8, 2016

Copy link
Copy Markdown
Contributor

I considered

  • Making offset and end_cursor read-only public properties on the subclass
  • Making a @property alias start_cursor that just returns the value of next_page_token
  • Making a @property alias limit that just returns the value of max_results

I elected not to make anything new public since the original implementation didn't.

@jonparrott PTAL as well (do you think this will be too jarring a change?).

@dhermes dhermes added api: datastore Issues related to the Datastore API. api: core labels Nov 8, 2016
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Nov 8, 2016
theacodes
theacodes reviewed Nov 8, 2016
View reviewed changes
Comment thread datastore/google/cloud/datastore/client.py

Using the query iterator's
:meth:`~google.cloud.datastore.query.Iterator.next_page` method:
Using the query iterator

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@dhermes
dhermes force-pushed the migrate-datastore-iterator branch 2 times, most recently from 22578a8 to 0bc7682 Compare November 8, 2016 18:02
@theacodes

theacodes commented Nov 8, 2016

Copy link
Copy Markdown
Contributor

@dhermes thanks, the same is clear and doesn't make a common use case too difficult. I'm cool with this.

dhermes added 13 commits November 8, 2016 12:36
@dhermes
Factoring protobuf construction out of datastore iterator.
d0abce8
@dhermes
Factoring query processing out of datastore iterator.
01e333f
@dhermes
Moving datastore iterator enum helpers into module scope.
a4af15a
@dhermes
Adding alternate datastore iterator class that inherits from base Ite…
79dca14 
…rator.
@dhermes
Adding helper for datastore iterator to convert entities.
f722637
@dhermes
Adding _build_protobuf() helper to datastore alt. iterator.
0f1ee53
@dhermes
Adding _process_query_results helper to alt. datastore iterator.
96fc53b
@dhermes
Implement _next_page() on alternate datastore iterator.
f2b828e
@dhermes
Removing original Iterator class in datastore.
a3f68a4
@dhermes
Renaming datastore AltIterator-->Iterator.
845efc9
@dhermes
Updating datastore system tests to use new iterator.
eec6e9f
@dhermes
Pylint fixes for datastore.
aaaa100 
Removing an unused import and also adding a class-level
attribute for ``next_page_token`` which gets set outside of the
constructor (Pylint seems to miss the fact that it gets set
in the constructor of the base class).
@dhermes
Adding doc snippet for paging with cursors.
7a97e00
@dhermes
dhermes force-pushed the migrate-datastore-iterator branch from 0bc7682 to 7a97e00 Compare November 8, 2016 20:39
@dhermes

dhermes commented Nov 8, 2016

Copy link
Copy Markdown
Contributor Author

@daspecster @tseaver PTAL

daspecster
daspecster reviewed Nov 8, 2016
View reviewed changes
Comment thread datastore/google/cloud/datastore/client.py
>>> query_iter.next_page_token is None
True

Under the hood this is doing:

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

daspecster
daspecster approved these changes Nov 8, 2016
View reviewed changes

@daspecster daspecster 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.

This LGTM to me as well.

@dhermes

dhermes commented Nov 9, 2016 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Going to merge (waiting waited 24h for other feedback)

@dhermes
dhermes merged commit 1abfcbe into googleapis:master Nov 9, 2016
@dhermes
dhermes deleted the migrate-datastore-iterator branch November 9, 2016 17:08
@dhermes dhermes mentioned this pull request Nov 14, 2016
Merged
@remcohaszing remcohaszing mentioned this pull request Nov 17, 2016
Closed
richkadel pushed a commit to richkadel/google-cloud-python that referenced this pull request May 6, 2017
@dhermes
Merge pull request googleapis#2706 from dhermes/migrate-datastore-ite…
5001fac 
…rator

Migrate datastore iterator to use base iterator class
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers
@theacodes theacodes theacodes left review comments
@daspecster daspecster daspecster approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@tseaver tseaver

Labels

api: core api: datastore Issues related to the Datastore 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.

5 participants

@dhermes @theacodes @daspecster @tseaver @googlebot