Skip to content

Add DML snippets - #1000

Open
morganchen12 wants to merge 27 commits into
mainfrom
mc/snippets-again
Open

morganchen12 wants to merge 27 commits into
mainfrom
mc/snippets-again

Conversation

@morganchen12

Copy link
Copy Markdown
Contributor

I will untangle the git history soon.

morganchen12 and others added 27 commits November 3, 2025 14:51
* add fid arg and token deprecate warning to Message class and add unit tests

* add fids to MulticastMessage and token deprecate warning and add unit tests

* Fix lint formatting error

* Position `tokens` first to guarantee backward compatibility for legacy positional arguments and add a unit test for it

* Extract multicast-to-message-list conversion logic into a private helper function

* Add docstring for the helper function

* Add unit tests for the async function, change deprecate message and address other review comments

* Fix error messages and MulticastMessage constructor and add mix types of fids and tokens

* Add integration tests for fid as argument

* Update integration tests error code for invalid fid target
…redential (#954)

Added a universe_domain property override to MockGoogleComputeEngineCredential in tests/testutils.py to prevent unexpected network requests to the GCE metadata server during test runs with newer versions of google-auth.
Updates the requirements on [pyjwt](https://github.com/jpadilla/pyjwt) to permit the latest version.
- [Release notes](https://github.com/jpadilla/pyjwt/releases)
- [Changelog](https://github.com/jpadilla/pyjwt/blob/master/CHANGELOG.rst)
- [Commits](jpadilla/pyjwt@2.10.1...2.12.1)

---
updated-dependencies:
- dependency-name: pyjwt
  dependency-version: 2.12.1
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Yvonne Pan <103622026+yvonnep165@users.noreply.github.com>
Updates the requirements on [google-cloud-firestore](https://github.com/googleapis/google-cloud-python) to permit the latest version.
- [Release notes](https://github.com/googleapis/google-cloud-python/releases)
- [Changelog](https://github.com/googleapis/google-cloud-python/blob/main/packages/google-cloud-documentai/CHANGELOG.md)
- [Commits](googleapis/google-cloud-python@google-cloud-firestore-v2.23.0...google-cloud-firestore-v2.27.0)

---
updated-dependencies:
- dependency-name: google-cloud-firestore
  dependency-version: 2.27.0
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Yvonne Pan <103622026+yvonnep165@users.noreply.github.com>
Updates the requirements on [pytest](https://github.com/pytest-dev/pytest) to permit the latest version.
- [Release notes](https://github.com/pytest-dev/pytest/releases)
- [Changelog](https://github.com/pytest-dev/pytest/blob/main/CHANGELOG.rst)
- [Commits](pytest-dev/pytest@8.2.2...8.4.2)

---
updated-dependencies:
- dependency-name: pytest
  dependency-version: 8.4.2
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Yvonne Pan <103622026+yvonnep165@users.noreply.github.com>
@morganchen12
morganchen12 requested a review from a team October 30, 2026 23:48

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a comprehensive set of pipeline snippets for Cloud Firestore in Python, covering joins, aggregations, DML operations, and document insertions. The feedback highlights a few key improvements: correcting an invalid argument usage in upsert_in_place by using .add_fields() before .raw_stage(), adding a clarifying comment in pipeline_force_table_scan regarding the lack of forceIndex support in the Python SDK, and refactoring the non-idiomatic magic indexing [1] in load_test_data to use tuple destructuring.

Comment on lines +664 to +670
results = (
db.pipeline()
.collection("cities")
.where(Field.of("name").equal("San Francisco"))
.raw_stage("upsert", Constant.of(880000).as_("population"))
.execute()
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

In upsert_in_place, passing Constant.of(880000).as_("population") directly as a positional argument to raw_stage is incorrect because the second parameter of raw_stage is options (which expects a dictionary), not a pipeline expression. To upsert specific fields, you should use .add_fields(...) before calling .raw_stage("upsert").

Suggested change
results = (
db.pipeline()
.collection("cities")
.where(Field.of("name").equal("San Francisco"))
.raw_stage("upsert", Constant.of(880000).as_("population"))
.execute()
)
results = (
db.pipeline()
.collection("cities")
.where(Field.of("name").equal("San Francisco"))
.add_fields(Constant.of(880000).as_("population"))
.raw_stage("upsert")
.execute()
)

Comment on lines +336 to +346
def pipeline_force_table_scan(db):
# [START pipeline_force_table_scan]
# Force Planner to only do a Full-Table Scan
results = (
db.pipeline()
.collection_group("customers")
.limit(100)
.execute()
)
# [END pipeline_force_table_scan]
return results

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The pipeline_force_table_scan snippet claims to force a full-table scan, but the code is just a standard query with no parameters or options to force a scan. Since forceIndex is not currently supported in the Python SDK, it is highly recommended to add an explanatory comment to avoid confusing users, similar to other snippets in the repository.

Suggested change
def pipeline_force_table_scan(db):
# [START pipeline_force_table_scan]
# Force Planner to only do a Full-Table Scan
results = (
db.pipeline()
.collection_group("customers")
.limit(100)
.execute()
)
# [END pipeline_force_table_scan]
return results
def pipeline_force_table_scan(db):
# [START pipeline_force_table_scan]
# Force Planner to only do a Full-Table Scan
# Note: forceIndex="primary" is not currently supported in the Python SDK.
results = (
db.pipeline()
.collection_group("customers")
.limit(100)
.execute()
)
# [END pipeline_force_table_scan]
return results

Comment on lines +59 to +65
rest1 = sf_restaurants.add(
{
"name": "Golden Gate Pizza",
"type": "pizza",
"owner_id": "Mario Rossi",
}
)[1]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using a magic index [1] to extract the DocumentReference from the tuple returned by CollectionReference.add() is less readable and non-idiomatic in Python. Consider using tuple destructuring (e.g., _, rest1 = ...) to improve code clarity. This pattern can be applied to all other restaurant additions in this function.

Suggested change
rest1 = sf_restaurants.add(
{
"name": "Golden Gate Pizza",
"type": "pizza",
"owner_id": "Mario Rossi",
}
)[1]
_, rest1 = sf_restaurants.add(
{
"name": "Golden Gate Pizza",
"type": "pizza",
"owner_id": "Mario Rossi",
}
)

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants