Skip to content

chore(bigtable): Added an internal batch completed callback to the data client mutations batcher - #18199

Draft
daniel-sanche wants to merge 3 commits into
shim/11-read-rows-data-clientfrom
shim/12-batcher-callback
Draft

chore(bigtable): Added an internal batch completed callback to the data client mutations batcher#18199
daniel-sanche wants to merge 3 commits into
shim/11-read-rows-data-clientfrom
shim/12-batcher-callback

Conversation

@daniel-sanche

@daniel-sanche daniel-sanche commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Migrating over @gkevinzheng PR from bigtable monorepo googleapis/python-bigtable#1308

Original description:

Changes made:

  • Refactored logic from Table.mutate_rows from producing a list of Status protos from a MutationsExceptionGroup
  • Added private keyword argument for a batch completion callback in the MutationsBatcher.
  • Added unit tests/system tests.

Additional Changes:

  • 504536b: incorporated gemini feedbck (guarded callback in try/catch, created distinct status objects)

Note to reviewers: This PR has already been reviewed and merged to a staging branch, with the intention of doing a single merge to main. We are now planning to slowly rollout these changes back to the main branch. Minimal re-review should be necessary

gkevinzheng and others added 2 commits August 21, 2026 14:57
…atcher (#1308)

**Changes made:**

- Refactored logic from `Table.mutate_rows` from producing a list of
`Status` protos from a `MutationsExceptionGroup`

- Added private keyword argument for a batch completion callback in the
MutationsBatcher.

- Added unit tests/system tests.

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

Copy link
Copy Markdown
Contributor

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 user batch completed callback to both the async and sync auto-generated MutationsBatcher classes, and refactors status-extraction logic into a shared helper function _get_statuses_from_mutations_exception_group in _helpers.py. The review feedback highlights critical issues with using list multiplication (e.g., [status_pb2.Status(...)] * size) which creates multiple references to the same mutable Status object, recommending list comprehensions instead. Additionally, the reviewer advises wrapping the user callback execution in a try-except block within the finally clause to prevent callback exceptions from masking other active exceptions, and suggests using module-level protobuf constants for consistency.

Comment thread packages/google-cloud-bigtable/google/cloud/bigtable/data/_helpers.py Outdated
Comment thread packages/google-cloud-bigtable/google/cloud/bigtable/table.py Outdated
Comment thread packages/google-cloud-bigtable/google/cloud/bigtable/table.py Outdated
Comment thread packages/google-cloud-bigtable/google/cloud/bigtable/data/_helpers.py Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants