Skip to content
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

fix: make sure that the proper exception type is bubbled up for ReadRows #696

Merged
merged 3 commits into from
Nov 18, 2022

Conversation

igorbernstein2
Copy link
Contributor

ReadRows (in client 1.x) bypasses gapic layer and use the raw grpc stubs. Which means that the errors are not being wrapped. Previously the this conversion would be handled in the Retry predicate, however this creates usability issues for end users that are trying to change the exceptions they want to retry on. This PR moves the wrapping a bit lower

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #<issue_number_goes_here> 🦕

ReadRows (in client 1.x) bypasses gapic layer and use the raw grpc stubs. Which means that the errors are not being wrapped. Previously the this conversion would be handled in the Retry predicate, however this creates usability issues for end users that are trying to change the exceptions they want to retry on. This PR moves the wrapping a bit lower
@product-auto-label product-auto-label bot added size: xs Pull request size is extra small. api: bigtable Issues related to the googleapis/python-bigtable API. labels Nov 17, 2022
@product-auto-label product-auto-label bot added size: s Pull request size is small. and removed size: xs Pull request size is extra small. labels Nov 18, 2022
@igorbernstein2 igorbernstein2 marked this pull request as ready for review November 18, 2022 04:05
@igorbernstein2 igorbernstein2 requested review from a team as code owners November 18, 2022 04:05
Copy link
Contributor

@mutianf mutianf left a comment

Choose a reason for hiding this comment

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

Is it possible to add a test on override the exception? Can we document how a customer can override the retry predicate?

@igorbernstein2 igorbernstein2 added the automerge Merge the pull request once unit tests and other checks pass. label Nov 18, 2022
@gcf-merge-on-green gcf-merge-on-green bot merged commit 5c72780 into googleapis:v1 Nov 18, 2022
@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Nov 18, 2022
@igorbernstein2 igorbernstein2 deleted the readrows-api-errors branch November 18, 2022 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: bigtable Issues related to the googleapis/python-bigtable API. size: s Pull request size is small.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants