Skip to content

Changing the AckReplyConsumer interface#1758

Merged
pongad merged 7 commits intogoogleapis:masterfrom
davidtorres:master
Mar 21, 2017
Merged

Changing the AckReplyConsumer interface#1758
pongad merged 7 commits intogoogleapis:masterfrom
davidtorres:master

Conversation

@davidtorres
Copy link
Copy Markdown

@garrettjonesgoogle @pongad I'm changing the interface of AckReplyConsumer because it really does not buy us anything to be able to accept an exception along with the ackreply. It actually makes the interface ambiguous, i.e what does it mean to accept an AckReply and a non null exception. And moreover I'd like to avoid methods in which you can pass null parameters.

…on to (googleapis#1645)".

Compression is not fully supported in gRPC, can't have it in the library
yet.

This reverts commit a599972.
…Consumer interface, it really is

no useful to be able to set an exception as ack reply, since the result
is the same as nack, if we ever require another result then we can just
add one more value to the AckReply enum.

Also adding a fail-safe catch so if the receiver ever throws an
exception we will interpret that as a nack and keep going.
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Mar 15, 2017
@pongad
Copy link
Copy Markdown
Contributor

pongad commented Mar 15, 2017

LGTM from me. @garrettjonesgoogle can +2

@garrettjonesgoogle
Copy link
Copy Markdown
Contributor

LGTM - but why are there no Travis test results here? @davidtorres could you push a PR update to rerun the tests?

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.009%) to 81.491% when pulling 3e906e6 on davidtorres:master into e9c05a5 on GoogleCloudPlatform:master.

@davidtorres
Copy link
Copy Markdown
Author

Tests are running, travis already passing. Thanks for reviewing.

@pongad
Copy link
Copy Markdown
Contributor

pongad commented Mar 21, 2017

LGTM, thank you!

@pongad pongad merged commit a2ca992 into googleapis:master Mar 21, 2017
meltsufin pushed a commit that referenced this pull request Dec 22, 2025
suztomo pushed a commit to suztomo/google-cloud-java that referenced this pull request Mar 11, 2026
suztomo pushed a commit to suztomo/google-cloud-java that referenced this pull request Mar 23, 2026
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants