-
Notifications
You must be signed in to change notification settings - Fork 1.6k
GH-3395: Use Original Key in Reply Record #3404
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
Fixes: spring-projects#3395 When using `ReplyingKafkaTemplate,` include the original key from the request record if such a key exists.
@@ -650,6 +650,13 @@ public Message<?> messageReturn(String in) { | |||
} | |||
---- | |||
|
|||
[[record-key-in-reply]] | |||
== Preserving Request Record Key in Reply |
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.
Why do we need a dedicated section for this single short paragraph?
Why it cannot be included in the end if the previous one?
At least it looks like both of them talks about similar context.
Where that Message
indeed covers what you try to say about multi-value reply.
this.replyTemplate.send(builder.build()); | ||
} | ||
|
||
protected void asyncSuccess(@Nullable Object result, String replyTopic, Message<?> source, | ||
boolean returnTypeMessage) { | ||
|
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.
This is really a good style to make code more readable.
There is might no reason to do that for single-like method header, but if it is multi-line, then better to separate body for easy review.
Please, revert this change and fix your IDE to preserver such a blank line on refactoring.
@Nullable | ||
private byte[] getReplyPartition(Message<?> source) { | ||
return source.getHeaders().get(KafkaHeaders.REPLY_PARTITION, byte[].class); | ||
} | ||
|
||
@Nullable | ||
private Object getReplyKeyFromRequest(Message<?> source) { | ||
return source.getHeaders().get(KafkaHeaders.RECEIVED_KEY); |
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 don't see a reason in this one-line method.
More over it feel like static
.
Way, either way: I'd prefer to not have it making me thinking "where else this method can be used" 😄
@@ -662,7 +663,6 @@ protected void acknowledge(@Nullable Acknowledgment acknowledgment) { | |||
|
|||
protected void asyncFailure(Object request, @Nullable Acknowledgment acknowledgment, Consumer<?, ?> consumer, | |||
Throwable t, Message<?> source) { | |||
|
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.
Why haven't you reverted this?
@@ -676,7 +676,6 @@ protected void asyncFailure(Object request, @Nullable Acknowledgment acknowledgm | |||
|
|||
protected void handleException(Object records, @Nullable Acknowledgment acknowledgment, Consumer<?, ?> consumer, | |||
Message<?> message, ListenerExecutionFailedException e) { | |||
|
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.
DITTO, please.
@@ -650,6 +650,12 @@ public Message<?> messageReturn(String in) { | |||
} | |||
---- | |||
|
|||
=== Original Record Key in Reply |
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.
Why do you still think that this short paragraph deserves its own title?
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 don't see a problem with a separate section for this, but I am neutral either way.
Fixes: #3395
When using
ReplyingKafkaTemplate,
include the original key from the request record if such a key exists.