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
feat: All changes required to change library to use ApiException and gapic internals #295
Conversation
aa951a8
to
8d2e164
Compare
...cloud-pubsublite/src/main/java/com/google/cloud/pubsublite/internal/CheckedApiException.java
Show resolved
Hide resolved
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.
Thanks for making this PR. I didn't review any of the tests. Here are some issues I found:
- This comment about setting stub on the publisher needs an update:
Line 56 in 7b6bd05
* <p>Custom batching settings and a custom GRPC stub can also be set. - I'm not sure if you intended
google-cloud-pubsublite/src/main/java/com/google/cloud/pubsublite/internal/testing/UnitTestExamples.java
to be there. - Sporadic class descriptions in
google-cloud-pubsublite/src/main/java/com/google/cloud/pubsublite/internal/wire/
. - Changes in
pubsublite-beam-io/
andpubsublite-kafka-shim/
should be their own PRs. - Changes to
grpc-google-cloud-pubsublite-v1/
should come from synthtool pulling from the source. - Update samples in a later PR.
- Also please resolve the conflicts with main.
I haven't tried the code to see how they impact samples myself. I can do that next week.
...cloud-pubsublite/src/main/java/com/google/cloud/pubsublite/internal/TrivialProxyService.java
Show resolved
Hide resolved
...oud-pubsublite/src/main/java/com/google/cloud/pubsublite/internal/ApiBackgroundResource.java
Show resolved
Hide resolved
...e-cloud-pubsublite/src/main/java/com/google/cloud/pubsublite/internal/wire/AssignerImpl.java
Outdated
Show resolved
Hide resolved
google-cloud-pubsublite/src/main/java/com/google/cloud/pubsublite/cloudpubsub/Subscriber.java
Outdated
Show resolved
Hide resolved
7b6bd05
to
beda6df
Compare
Replies to each point below.
|
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.
- It's in the public documentation, either we add more documentation there, or we choose to hide it. Can be in a later PR.
- Ok. Make sure to make a new release soon after this PR is merged then.
This converts the internals of the client library to use GAPIC generated clients, as well as the api surface to raise ApiException as opposed to grpc StatusException.
fixes #180
fixes #148
This does not throw RESOURCE_EXHAUSTED at high partition counts with the following code when removing RESOURCE_EXHAUSTED from the list of retryable errors: