chore(firestore): prep for firestore pipelines GA#16549
chore(firestore): prep for firestore pipelines GA#16549daniel-sanche wants to merge 3 commits intomainfrom
Conversation
![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 removes preview API warnings from several Firestore pipeline modules and adds support for an optional options dictionary in the raw_stage method of the pipeline. Unit tests were also added to verify the new functionality. Feedback was provided regarding a missing type hint import for Value in base_pipeline.py and the placement of imports within test functions, which should ideally be moved to the top of the files per PEP 8.
| self, | ||
| name: str, | ||
| *params: Expression, | ||
| options: dict[str, Expression | Value] | None = None, |
There was a problem hiding this comment.
The type hint Value is used here but it does not appear to be imported in this file. If Value is a type alias defined in google.cloud.firestore_v1.pipeline_expressions, it should be imported alongside Expression to avoid a NameError at runtime, especially if type hints are evaluated.
| from google.cloud.firestore_v1.base_vector_query import Field | ||
| from google.cloud.firestore_v1.pipeline_stages import RawStage |
There was a problem hiding this comment.
It is generally better practice to place imports at the top of the file rather than inside a test function, unless there is a specific reason to defer the import (e.g., avoiding circular dependencies). Also, consider if Field should be imported from google.cloud.firestore_v1.pipeline_expressions for consistency with the pipeline API being tested.
References
- PEP 8: Imports are always put at the top of the file, just after any module comments and docstrings, and before module globals and constants. (link)
| from google.cloud.firestore_v1.base_vector_query import Field | ||
| from google.cloud.firestore_v1.pipeline_stages import RawStage |
There was a problem hiding this comment.
Similar to the async pipeline tests, these imports should ideally be moved to the top of the file. Additionally, ensure that importing Field from base_vector_query is intentional and consistent with how it's used in the pipeline implementation.
References
- PEP 8: Imports are always put at the top of the file, just after any module comments and docstrings, and before module globals and constants. (link)
1 participant
WIP leading up to next release