chore: complete google-cloud-firestore migration to librarian#16742
chore: complete google-cloud-firestore migration to librarian#16742jskeet wants to merge 4 commits intogoogleapis:mainfrom
Conversation
|
@parthea I've split this PR into multiple commits to make it clearer - it looks like all the changes come from either reformatting or regenerating with legacylibrarian. The deleted lines are worrying though - I'm not sure whether those are breaking changes or not. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request transitions the google-cloud-firestore library to automated generation, updates the supported Python versions to 3.9 and above, and introduces an expiration_offset field to the TTL configuration. The update also removes several Pipeline and Vector Search exports from the public API, which appears to be a regression that could break existing user code. Additionally, there is an inconsistency between the TtlConfig docstring, which states that sub-second precision is rejected, and a unit test that includes nanoseconds in the offset.
I am having trouble creating individual review comments. Click here to see my feedback.
packages/google-cloud-firestore/google/cloud/firestore/init.py (33-34)
The removal of AsyncPipeline and AsyncPipelineStream (along with other Pipeline-related exports like Pipeline, FindNearestOptions, and Ordering later in this file) appears to be a regression. These features were recently added to the Firestore SDK, and their removal from the public API will break existing user code. Please verify if these components should be preserved during the librarian migration.
packages/google-cloud-firestore/google/cloud/firestore_admin_v1/types/field.py (148-150)
The docstring states that "Values more precise than seconds are rejected" for expiration_offset. However, the corresponding unit test in test_firestore_admin.py (line 20834) includes a nanos value ("nanos": 543). This inconsistency should be resolved: if the API indeed rejects sub-second precision, the test data should be updated to reflect this restriction; otherwise, the docstring should be corrected.
|
It looks like a lot of the changes in |
chalmerlowe
left a comment
chalmerlowe
left a comment
There was a problem hiding this comment.
LGTM, pending a fix for the failing system test.
jskeet
added
the
do not merge
| AsyncCollectionReference, | ||
| AsyncDocumentReference, | ||
| AsyncPipeline, | ||
| AsyncPipelineStream, |
| from google.cloud.firestore_v1.async_client import AsyncClient | ||
| from google.cloud.firestore_v1.async_collection import AsyncCollectionReference | ||
| from google.cloud.firestore_v1.async_document import AsyncDocumentReference | ||
| from google.cloud.firestore_v1.async_pipeline import AsyncPipeline |
There was a problem hiding this comment.
Right, that was my thinking too. Will create a change for the post-processing file tomorrow.
3 participants
No description provided.