-
Notifications
You must be signed in to change notification settings - Fork 1.7k
chore(firestore): prep for firestore pipelines GA #16549
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -448,3 +448,17 @@ def test_async_pipeline_aggregate_with_groups(): | |
| assert isinstance(result_ppl.stages[0], stages.Aggregate) | ||
| assert list(result_ppl.stages[0].groups) == [Field.of("author")] | ||
| assert list(result_ppl.stages[0].accumulators) == [Field.of("title")] | ||
|
|
||
|
|
||
| def test_async_pipeline_raw_stage_with_options(): | ||
| from google.cloud.firestore_v1.base_vector_query import Field | ||
| from google.cloud.firestore_v1.pipeline_stages import RawStage | ||
|
Comment on lines
+454
to
+455
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 References
|
||
|
|
||
| start_ppl = _make_async_pipeline() | ||
| result_ppl = start_ppl.raw_stage( | ||
| "stage_name", Field.of("n"), options={"key": "val"} | ||
| ) | ||
| assert len(start_ppl.stages) == 0 | ||
| assert len(result_ppl.stages) == 1 | ||
| assert isinstance(result_ppl.stages[0], RawStage) | ||
| assert result_ppl.stages[0].options == {"key": "val"} | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -437,3 +437,17 @@ def test_pipeline_aggregate_with_groups(): | |
| assert isinstance(result_ppl.stages[0], stages.Aggregate) | ||
| assert list(result_ppl.stages[0].groups) == [Field.of("author")] | ||
| assert list(result_ppl.stages[0].accumulators) == [Field.of("title")] | ||
|
|
||
|
|
||
| def test_pipeline_raw_stage_with_options(): | ||
| from google.cloud.firestore_v1.base_vector_query import Field | ||
| from google.cloud.firestore_v1.pipeline_stages import RawStage | ||
|
Comment on lines
+443
to
+444
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar to the async pipeline tests, these imports should ideally be moved to the top of the file. Additionally, ensure that importing References
|
||
|
|
||
| start_ppl = _make_pipeline() | ||
| result_ppl = start_ppl.raw_stage( | ||
| "stage_name", Field.of("n"), options={"key": "val"} | ||
| ) | ||
| assert len(start_ppl.stages) == 0 | ||
| assert len(result_ppl.stages) == 1 | ||
| assert isinstance(result_ppl.stages[0], RawStage) | ||
| assert result_ppl.stages[0].options == {"key": "val"} | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The type hint
Valueis used here but it does not appear to be imported in this file. IfValueis a type alias defined ingoogle.cloud.firestore_v1.pipeline_expressions, it should be imported alongsideExpressionto avoid aNameErrorat runtime, especially if type hints are evaluated.