-
Notifications
You must be signed in to change notification settings - Fork 327
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
Pub/Sub publish callbacks with original Message awareness #482
Conversation
cc/ @nvolynets |
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.
Looks good, just some minor naming comments.
@@ -130,11 +134,31 @@ public void setPublishTimeout(long timeoutMillis) { | |||
/** | |||
* Set the callback to be activated when the publish call resolves. | |||
* @param publishCallback callback for the publish future | |||
* @deprecated Use {@code setSuccessCallback()} and {@code setFailureCallback()} instead. |
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.
You can use {@link
here.
/** | ||
* Set callback (can be a lambda) for processing the published message ID and the original {@code Message} after the | ||
* message was successfully published. | ||
* @param cb callback accepting a {@code String} message ID and the original {@code Message}. |
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.
cb
seems to be different from the long-form style. successCallback
?
* of failure. | ||
* @param cb callback accepting a {@code Throwable} and a {@code Message}. | ||
*/ | ||
public void setFailureCallback(FailureCallback cb) { |
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.
failureCallback
?
It is possible to set user-defined callbacks for the `publish()` call in `PubSubMessageHandler` through the `setPublishFutureCallback()` method. | ||
These are useful to process the message ID, in case of success, or the error if any was thrown. | ||
It is possible to set user-defined callbacks for the `publish()` call in `PubSubMessageHandler` through the `setSuccessCallback()` and `setFailureCallback()` methods (either one or both may be set). | ||
These give access to the Pub/Sub publish message ID in case of success, or the root cause exception in case of error. |
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.
"message ID" always seems confusing to me. What we're really talking about is the "message ack ID", right?
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.
The confusion goes down to proto level. Publish operation returns server-assigned message ID. But when we acknowledge, it's called "ack ID".
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.
oh wait, no, those are different things! When we publish we get back message ID. But when we acknowledge pulled messages, we get ack ID. So I was confused in the same place.
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.
When you pull a message, you get an ack ID with it, which you can use to acknowledge it.
I don't think you get anything back when you acknowledge a message.
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.
Right, but we are not pulling a message. We are publishing a message, so we are getting that message's ID back.
Kudos, SonarCloud Quality Gate passed! |
…dPlatform#482) Two new callbacks provided for SI Pub/Sub async publish operation: setSuccessCallback() and setFailureCallback(). Second parameter to both is the original Message for consistency, and to enable forwarding to downstream channels.
Bumps [reactor-bom](https://github.com/reactor/reactor) from 2020.0.14 to 2020.0.15. - [Release notes](https://github.com/reactor/reactor/releases) - [Commits](reactor/reactor@2020.0.14...2020.0.15) --- updated-dependencies: - dependency-name: io.projectreactor:reactor-bom dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Two callbacks provided:
setSuccessCallback()
andsetFailureCallback()
.Second parameter to both is the original
Message
for consistency, and to enable forwarding to downstream channels.Fixes #467.