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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants