feat: Added read_time as a parameter to various calls (synchronous/base classes) #1050
Conversation
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
daniel-sanche
requested changes
May 21, 2025
| @@ -365,6 +365,8 @@ def get( | |||
| transaction=None, | |||
| retry: retries.Retry | object | None = gapic_v1.method.DEFAULT, | |||
| timeout: float | None = None, | |||
| *, | |||
| read_time: Optional[datetime.datetime] = None, | |||
There was a problem hiding this comment.
nit: The other arguments use | None instead of Optional
google/cloud/firestore_v1/client.py
Outdated
| @@ -261,6 +269,8 @@ def collections( | |||
| self, | |||
| retry: retries.Retry | object | None = gapic_v1.method.DEFAULT, | |||
| timeout: float | None = None, | |||
| *, | |||
| read_time: Optional[datetime.datetime] = None, | |||
There was a problem hiding this comment.
What about the async versions of these classes?
There was a problem hiding this comment.
Would adding the async stuff bloat this PR? I was going to add the async options in another PR, similar to what was done with ExplainOptions.
| assert set(collection.list_documents()) == set() | ||
|
|
||
| data1 = {"foo": "bar"} | ||
| update_time1, document_ref1 = collection.add(data1) |
There was a problem hiding this comment.
It seems like all the tests are using times directly read back from the API. It might be good to have some explictly-constructed datetimes as well?
Edit: It looks like the unit tests cover this. Might be good to have as a system test too, but I don't think that matters too much, as long as its tested somewhere
There was a problem hiding this comment.
Added a parameter to generate datetime.datetime.now objects.
|
LGTM, but let's hold off on merging it until the async is ready |
daniel-sanche
previously approved these changes
May 23, 2025
…#1059) * feat: Added read_time as a parameter to various calls (async classes) * used TYPE_CHECKING; fixed unit tests * linting + fixing cover * final linting
daniel-sanche
previously approved these changes
Jun 3, 2025
fix no cover comment
daniel-sanche
previously approved these changes
Jun 3, 2025
daniel-sanche
approved these changes
Jun 4, 2025
daniel-sanche
added a commit
that referenced
this pull request
Jun 9, 2025
commit 0ff25c1 Merge: dc5b5ac 22b558c Author: Daniel Sanche <sanche@google.com> Date: Mon Jun 9 14:38:14 2025 -0700 Merge branch 'pipeline_queries_1_stubs' into pipeline_queries_2_query_parity commit 22b558c Merge: b46bdc1 dc808b5 Author: Daniel Sanche <d.sanche14@gmail.com> Date: Mon Jun 9 14:16:25 2025 -0700 Merge branch 'pipeline_queries_approved' into pipeline_queries_1_stubs commit dc808b5 Merge: 3f9b65f 7f96290 Author: Daniel Sanche <d.sanche14@gmail.com> Date: Mon Jun 9 13:43:53 2025 -0700 Merge branch 'main' into pipeline_queries_approved commit 7f96290 Author: Daniel Sanche <sanche@google.com> Date: Fri Jun 6 11:14:43 2025 -0700 chore: enable mypy testing (#1057) commit d8e3af1 Author: Kevin Zheng <147537668+gkevinzheng@users.noreply.github.com> Date: Wed Jun 4 13:59:27 2025 -0400 feat: Added read_time as a parameter to various calls (synchronous/base classes) (#1050) * feat: Added read_time as a parameter to various calls (synchronous/base classes) * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * fixed tests + added system tests * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * Removed specific system test assertions * added system test with python datetimes * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * revised type hints * linting * feat: Added read_time as a parameter to various calls (async classes) (#1059) * feat: Added read_time as a parameter to various calls (async classes) * used TYPE_CHECKING; fixed unit tests * linting + fixing cover * final linting * TYPE_CHECKING * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * Update client.py fix no cover comment * fixed async system test --------- Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com> Co-authored-by: Daniel Sanche <sanche@google.com> commit 437e233 Author: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> Date: Wed May 28 09:56:14 2025 -0700 chore(main): release 2.21.0 (#1055) Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> commit 0fb4f2d Author: Daniel Sanche <sanche@google.com> Date: Fri May 23 14:31:46 2025 -0700 chore: update renovate.json (#1058) commit 6c81626 Author: Jing <lscmirror@gmail.com> Date: Fri May 23 09:46:58 2025 -0700 feat: Support Sequence[float] as query_vector in FindNearest (#908) commit b01a03c Author: Daniel Sanche <sanche@google.com> Date: Thu May 22 17:42:51 2025 -0700 chore(tests): system test for unicode characters (#1003) commit f8bf2af Author: Daniel Sanche <sanche@google.com> Date: Thu May 22 16:16:19 2025 -0700 chore: add java 21 to fix emulator tests (#1056) commit 5a279b2 Author: gcf-owl-bot[bot] <78513119+gcf-owl-bot[bot]@users.noreply.github.com> Date: Wed May 21 06:35:29 2025 -0400 chore: Update gapic-generator-python to 1.25.0 (#1043) * chore: Update gapic-generator-python to 1.25.0 PiperOrigin-RevId: 755914147 Source-Link: googleapis/googleapis@97a83d7 Source-Link: googleapis/googleapis-gen@a9977ef Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiYTk5NzdlZmVkYzgzNmNjZWNlMWYwMWQ1MjliMDMxNWUxZWZlNTJhZCJ9 * 🦉 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> commit 043d9ef Author: Jeff Verkoeyen <jeff@clutch.engineering> Date: Tue May 20 17:37:28 2025 -0700 fix: Add missing DocumentReference return value to .document (#1053) commit dc5b5ac Merge: 5beee36 b46bdc1 Author: Daniel Sanche <sanche@google.com> Date: Thu May 15 16:49:40 2025 -0700 Merge branch 'pipeline_queries_1_stubs' into pipeline_queries_2_query_parity commit 5beee36 Author: Daniel Sanche <d.sanche14@gmail.com> Date: Thu May 15 16:47:24 2025 -0700 ran blacken commit c8cfcee Author: Daniel Sanche <d.sanche14@gmail.com> Date: Thu May 15 16:44:30 2025 -0700 added tests commit f22f11e Author: Daniel Sanche <d.sanche14@gmail.com> Date: Thu May 15 16:35:40 2025 -0700 support tuples in pipelin_source.collection commit 63b83b8 Author: Daniel Sanche <d.sanche14@gmail.com> Date: Thu May 15 15:43:31 2025 -0700 added tests for expressions commit c4cd995 Author: Daniel Sanche <d.sanche14@gmail.com> Date: Tue May 13 17:31:55 2025 -0700 added tests for filter conditions commit 2775da2 Author: Daniel Sanche <d.sanche14@gmail.com> Date: Tue May 13 17:04:20 2025 -0700 improve FilterCondition repr commit a9368b3 Author: Daniel Sanche <d.sanche14@gmail.com> Date: Tue May 13 15:58:09 2025 -0700 added stages unit tests commit 28fd42d Author: Daniel Sanche <d.sanche14@gmail.com> Date: Tue May 13 15:28:04 2025 -0700 added tests commit b46bdc1 Author: Daniel Sanche <d.sanche14@gmail.com> Date: Tue May 13 15:02:46 2025 -0700 fixed test issues commit d2babd2 Merge: 2d286bb 3f9b65f Author: Daniel Sanche <d.sanche14@gmail.com> Date: Tue May 13 11:25:51 2025 -0700 Merge branch 'pipeline_queries_approved' into pipeline_queries_1_stubs commit 3f9b65f Author: Daniel Sanche <d.sanche14@gmail.com> Date: Thu May 8 14:07:16 2025 -0700 chore: updated gapic layer for execute_query
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is a rewrite of #1013 that's basically the same PR. After a rebase added unnecessary commits to the PR, I decided to rewrite it.
Fixes #775