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

Query should expose the value of more_result after query.fetch is called #423

Closed
silvolu opened this issue Dec 17, 2014 · 8 comments · Fixed by #425
Closed

Query should expose the value of more_result after query.fetch is called #423

silvolu opened this issue Dec 17, 2014 · 8 comments · Fixed by #425
Assignees
@tseaver
Labels
api: datastore Issues related to the Datastore API. 🚨 This issue needs some love. triage me I really want to be triaged.

Comments

@silvolu
Copy link
Contributor

silvolu commented Dec 17, 2014

A user that runs query.fetch(5) and gets 4 results will not know if they should call it again or if that is all the results.
@silvolu silvolu added api: datastore Issues related to the Datastore API. fix-asap labels Dec 17, 2014
@dhermes
Copy link
Contributor

dhermes commented Dec 17, 2014

We're getting mixed signals on this.

We were told in #280 that more_results is broken in the API and it is noted in our code. This note was added in #287 and there is an accompanying discussion.

@tseaver
Copy link
Contributor

tseaver commented Dec 17, 2014

I don't believe it should be returned to the user: that kind of state belongs on the query instance (like the cursor value).

We could choose to save the more_results value on the query (along with the end_cursor), and expose it via a method or property, even though we know (for now) that the back-end borks it: after all, they might fix it in the future.

@silvolu
Copy link
Contributor Author

silvolu commented Dec 17, 2014

Yeah, exposing sounds better. Fixing the title of the issue.

@silvolu silvolu changed the title query.fetch should return the value of more_result Query should expose the value of more_result after query.fetch is called Dec 17, 2014
@tseaver tseaver self-assigned this Dec 17, 2014
@dhermes
Copy link
Contributor

dhermes commented Dec 17, 2014

I don't think it belongs on the Query object. It is a result of a query, not a query itself (response, not request).

Two possible solutions that come to mind

  • Create a separate method fetch_page which returns more_results. (I prefer this)
  • Add a flag to fetch which allows a tuple of things to be returned instead of just the fetched entities.

@tseaver tseaver mentioned this issue Dec 17, 2014
Merged
@tseaver
Copy link
Contributor

tseaver commented Dec 17, 2014

@dhermes We already store the cursor on the query; how is more_results different?

@dhermes
Copy link
Contributor

dhermes commented Dec 17, 2014

I'm saying we shouldn't be storing cursor on the query, I planned on changing this, so adding more_results would be piling on.

@tseaver
Copy link
Contributor

tseaver commented Dec 17, 2014

Where are you planning to track that state? In a cursor object returned by fetch? We could merge #425 and then move the two bits of state together.

@dhermes
Copy link
Contributor

dhermes commented Dec 17, 2014

The state should not be tracked, it should be returned and the user may use it as is.

Your plan of merge SGTM. Reviewing now.

tseaver added a commit that referenced this issue Dec 17, 2014
@tseaver
Merge pull request #425 from tseaver/423-expose_query_more_results
1b9f1c2 
Fix #423:  Expose the 'more_results' value fetched from the back-end.
dhermes added a commit to dhermes/google-cloud-python that referenced this issue Dec 17, 2014
@dhermes
Removing cursor and more results from query object.
715f8d6 
This is a follow up to a discussion in googleapis#423 and googleapis#425.

Also
- Adding fetch_page() method to return the needed properties
  no longer available on the Query object.
- Made fetch_page() fail if more_results is `NOT_FINISHED`, which
  seems to be intended for batched requests.
- Added start_cursor and end_cursor properties to Query.
- Updated docstrings to reflect change
- Updated tests to ensure the returned cursor was always set.
- Streamlined fetch()/fetch_page() tests to run via a single
  tweakable helper.
- Updated a datastore.connection test to construct and return an
  initialized QueryResultBatch to simulate a valid cursor.
- Updated regression.clear_datastore and regression.datastore to
  reflect changed API surface.
@dhermes dhermes mentioned this issue Dec 17, 2014
Merged
dhermes added a commit to dhermes/google-cloud-python that referenced this issue Dec 17, 2014
@dhermes
Removing cursor and more results from query object.
2d40365 
This is a follow up to a discussion in googleapis#423 and googleapis#425.

Also
- Adding fetch_page() method to return the needed properties
  no longer available on the Query object.
- Made fetch_page() fail if more_results is `NOT_FINISHED`, which
  seems to be intended for batched requests.
- Added start_cursor and end_cursor properties to Query.
- Updated docstrings to reflect change
- Updated tests to ensure the returned cursor was always set.
- Streamlined fetch()/fetch_page() tests to run via a single
  tweakable helper.
- Updated a datastore.connection test to construct and return an
  initialized QueryResultBatch to simulate a valid cursor.
- Updated regression.clear_datastore and regression.datastore to
  reflect changed API surface.
dhermes added a commit to dhermes/google-cloud-python that referenced this issue Dec 18, 2014
@dhermes
Removing cursor and more results from query object.
78d0dde 
This is a follow up to a discussion in googleapis#423 and googleapis#425.

Also
- Adding fetch_page() method to return the needed properties
  no longer available on the Query object.
- Made fetch_page() fail if more_results is `NOT_FINISHED`, which
  seems to be intended for batched requests.
- Added start_cursor and end_cursor properties to Query.
- Updated docstrings to reflect change
- Updated tests to ensure the returned cursor was always set.
- Streamlined fetch()/fetch_page() tests to run via a single
  tweakable helper.
- Updated a datastore.connection test to construct and return an
  initialized QueryResultBatch to simulate a valid cursor.
- Updated regression.clear_datastore and regression.datastore to
  reflect changed API surface.
@jgeewax jgeewax modified the milestone: Datastore Stable Jan 30, 2015
@yoshi-automation yoshi-automation added triage me I really want to be triaged. 🚨 This issue needs some love. labels Apr 7, 2020
atulep pushed a commit that referenced this issue Apr 6, 2023
@renovate-bot @gcf-owl-bot @parthea
chore(deps): update all dependencies (#423)
587c546 
* chore(deps): update all dependencies

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

* revert

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Co-authored-by: Anthonios Partheniou <partheniou@google.com>
atulep pushed a commit that referenced this issue Apr 6, 2023
@renovate-bot @gcf-owl-bot @parthea
chore(deps): update all dependencies (#423)
75f6d97 
* chore(deps): update all dependencies

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

* revert

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Co-authored-by: Anthonios Partheniou <partheniou@google.com>
atulep pushed a commit that referenced this issue Apr 18, 2023
@renovate-bot @gcf-owl-bot @parthea
chore(deps): update all dependencies (#423)
222da05 
* chore(deps): update all dependencies

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

* revert

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Co-authored-by: Anthonios Partheniou <partheniou@google.com>
parthea pushed a commit that referenced this issue Jun 4, 2023
@renovate-bot
chore(deps): update dependency google-cloud-documentai to v2.4.0 (#423)
9eb32a0
parthea pushed a commit that referenced this issue Jun 4, 2023
@renovate-bot
chore(deps): update dependency google-cloud-pubsub to v2.13.12 (#423)
e53dadf
parthea added a commit that referenced this issue Jul 6, 2023
@gcf-owl-bot @parthea
feat: enable "rest" transport in Python for services supporting numer…
98bd8fc 
…ic enums (#423)

* feat: enable "rest" transport in Python for services supporting numeric enums

PiperOrigin-RevId: 508143576

Source-Link: googleapis/googleapis@7a702a9

Source-Link: googleapis/googleapis-gen@6ad1279
Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiNmFkMTI3OWMwZTdhYTc4N2FjNmI2NmM5ZmQ0YTIxMDY5MmVkZmZjZCJ9

* fix typo

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

---------

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Co-authored-by: Anthonios Partheniou <partheniou@google.com>
parthea pushed a commit that referenced this issue Aug 15, 2023
@nicain
fix: Update detect_intent_synthesize_tts_response.py (#423)
f842083 
* fix: Update detect_intent_synthesize_tts_response.py

* Update detect_intent_synthesize_tts_response.py
parthea pushed a commit that referenced this issue Sep 20, 2023
@renovate-bot
chore(deps): update dependency google-cloud-retail to v1.16.2 (#423)
0f5eb98
parthea pushed a commit that referenced this issue Sep 22, 2023
@renovate-bot
chore(deps): update dependency google-cloud-pubsub to v2.13.12 (#423)
2e5173d
parthea added a commit that referenced this issue Sep 22, 2023
@parthea
fix(deps): require protobuf <4.0.0dev (#423)
3ea672a
parthea pushed a commit that referenced this issue Sep 22, 2023
@release-please
chore(main): release 3.8.1 (#423)
7809d59 
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
parthea pushed a commit that referenced this issue Oct 21, 2023
@renovate-bot
chore(deps): update dependency google-cloud-bigquery to v3.1.0 (#423)
3174b36
parthea pushed a commit that referenced this issue Oct 21, 2023
@gcf-owl-bot
chore: Update gapic-generator-python to v1.11.4 (#423)
3e3c72d 
* chore: Update gapic-generator-python to v1.11.4

PiperOrigin-RevId: 547897126

Source-Link: googleapis/googleapis@c09c75e

Source-Link: googleapis/googleapis-gen@45e0ec4
Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiNDVlMGVjNDM0MzUxN2NkMGFhNjZiNWNhNjQyMzJhMTgwMmMyZjk0NSJ9

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

---------

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
parthea added a commit that referenced this issue Oct 21, 2023
@gcf-owl-bot @parthea
chore: Update gapic-generator-python to v1.11.9 (#423)
17d9400 
* chore: Update gapic-generator-python to v1.11.7

PiperOrigin-RevId: 573230664

Source-Link: googleapis/googleapis@93beed3

Source-Link: googleapis/googleapis-gen@f4a4eda
Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiZjRhNGVkYWE4MDU3NjM5ZmNmNmFkZjkxNzk4NzIyODBkMWE4ZjY1MSJ9

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

* chore: Update gapic-generator-python to v1.11.8

PiperOrigin-RevId: 574178735

Source-Link: googleapis/googleapis@7307199

Source-Link: googleapis/googleapis-gen@ce3af21
Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiY2UzYWYyMWI3YzU1OWE4N2MyYmVmYzA3NmJlMGUzYWVkYTNhMjZmMCJ9

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

* chore: Update gapic-generator-python to v1.11.9

PiperOrigin-RevId: 574520922

Source-Link: googleapis/googleapis@5183984

Source-Link: googleapis/googleapis-gen@a59af19
Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiYTU5YWYxOWQ0YWM2NTA5ZmFlZGYxY2MzOTAyOTE0MWI2YTViODk2OCJ9

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

* update post processor image; remove unused files

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

---------

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Co-authored-by: Anthonios Partheniou <partheniou@google.com>
parthea added a commit that referenced this issue Oct 21, 2023
@renovate-bot @gcf-owl-bot @parthea
chore(deps): update all dependencies (#423)
aac2d7b 
* chore(deps): update all dependencies

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

* revert

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Co-authored-by: Anthonios Partheniou <partheniou@google.com>
parthea pushed a commit that referenced this issue Oct 21, 2023
@gcf-owl-bot
build(deps): [autoapprove] bump cryptography from 41.0.0 to 41.0.2 (#423
0b1b81d 
)

Source-Link: https://togithub.com/googleapis/synthtool/commit/d6103f4a3540ba60f633a9e25c37ec5fe7e6286d
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:39f0f3f2be02ef036e297e376fe3b6256775576da8a6ccb1d5eeb80f4c8bf8fb
parthea added a commit that referenced this issue Oct 22, 2023
@renovate-bot @gcf-owl-bot @parthea
chore(deps): update all dependencies (#423)
43c27f1 
* chore(deps): update all dependencies

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

* revert

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Co-authored-by: Anthonios Partheniou <partheniou@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tseaver tseaver

Labels
api: datastore Issues related to the Datastore API. 🚨 This issue needs some love. triage me I really want to be triaged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix #423: Expose the 'more_results' value fetched from the back-end.
5 participants
@jgeewax @tseaver @dhermes @silvolu @yoshi-automation