This repository was archived by the owner on Apr 1, 2026. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 65
feat: Added a batch completed callback to the data client mutations batcher #1308
Merged
Merged
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
0b225ac
feat: Added a batch completed callback to the data client mutations b…
gkevinzheng 4e3d8bb
Added input vvalidation + unit tests
gkevinzheng f03977c
linting
gkevinzheng 701eebb
fixed mypy
gkevinzheng 8320f9d
Fixed helper function and mutations batcher constructor
gkevinzheng 9069d34
Merge branch 'v3_staging' into batcher-callback
gkevinzheng 0741d0b
None case in _get_status
gkevinzheng 8993539
Fixed unit tests
gkevinzheng 20fb4cb
fixed system tests
gkevinzheng c4da6eb
linting
gkevinzheng File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,6 +26,10 @@ | |
| from google.api_core import retry as retries | ||
| from google.api_core.retry import RetryFailureReason | ||
| from google.cloud.bigtable.data.exceptions import RetryExceptionGroup | ||
| from google.cloud.bigtable.data.exceptions import MutationsExceptionGroup | ||
| from google.rpc import code_pb2 | ||
| from google.rpc import status_pb2 | ||
|
|
||
|
|
||
| if TYPE_CHECKING: | ||
| import grpc | ||
|
|
@@ -224,6 +228,66 @@ def _align_timeouts(operation: float, attempt: float | None) -> tuple[float, flo | |
| return operation, final_attempt | ||
|
|
||
|
|
||
| def _get_statuses_from_mutations_exception_group( | ||
| exc_group: MutationsExceptionGroup, batch_size: int | ||
| ) -> list[status_pb2.Status]: | ||
| """ | ||
| Helper function that populates a list of Status objects with exception information from | ||
| the exception group. | ||
|
|
||
| Args: | ||
| exc_group: The exception group from a mutate rows operation | ||
| batch_size: How many RowMutationGroups were provided to the batch | ||
| Returns: | ||
| list[status_pb2.Status]: A list of Status proto objects | ||
| """ | ||
| # We exception handle as follows: | ||
| # | ||
| # 1. Each exception in the error group is a FailedMutationEntryError, and its | ||
| # cause is either a singular exception or a RetryExceptionGroup consisting of | ||
| # multiple exceptions. | ||
| # | ||
| # 2. In the case of a singular exception, if the error does not have a gRPC status | ||
| # code, we return a status code of UNKNOWN. | ||
| # | ||
| # 3. In the case of a RetryExceptionGroup, we use terminal exception in the exception | ||
| # group and process that. | ||
| statuses = [status_pb2.Status(code=code_pb2.OK)] * batch_size | ||
| for error in exc_group.exceptions: | ||
| if isinstance(error.index, int) and 0 <= error.index < len(statuses): | ||
| cause = error.__cause__ | ||
| if isinstance(cause, RetryExceptionGroup): | ||
| statuses[error.index] = _get_status(cause.exceptions[-1]) | ||
| else: | ||
| statuses[error.index] = _get_status(cause) | ||
| return statuses | ||
|
|
||
|
|
||
| def _get_status(exc: Exception) -> status_pb2.Status: | ||
|
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 looks like this can be called with None, if error.__cause__ is None? Or is that not possible? |
||
| """ | ||
| Helper function that returns a Status object corresponding to the given exception. | ||
|
|
||
| Args: | ||
| exc: An exception to be converted into a Status. | ||
| Returns: | ||
| status_pb2.Status: A Status proto object. | ||
| """ | ||
| if ( | ||
| isinstance(exc, core_exceptions.GoogleAPICallError) | ||
| and exc.grpc_status_code is not None | ||
| ): | ||
| return status_pb2.Status( # type: ignore[unreachable] | ||
| code=exc.grpc_status_code.value[0], | ||
| message=exc.message, | ||
| details=exc.details, | ||
| ) | ||
|
|
||
| return status_pb2.Status( | ||
| code=code_pb2.Code.UNKNOWN, | ||
| message=str(exc), | ||
|
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. If exc can be None, we should probably change this message? |
||
| ) | ||
|
|
||
|
|
||
| def _validate_timeouts( | ||
| operation_timeout: float, attempt_timeout: float | None, allow_none: bool = False | ||
| ): | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Do we need to do an extra check here to make sure cause.exceptions has items? I can't remember how that works.
If so, there could be potential for an IndexError here
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.
I did some testing and Python prevents you from initializing an exception group without items, so we probably don't need to worry about the empty case.
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.
Yeah, looks like that's consistent with our custom object:
python-bigtable/google/cloud/bigtable/data/exceptions.py
Line 81 in c3ea54e