AndrewJSchofield commented on code in PR #18419:
URL: https://github.com/apache/kafka/pull/18419#discussion_r1906889857
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/ShareConsumeRequestManager.java:
##########
@@ -1169,15 +1169,17 @@ class ResultHandler {
* signal the completion when all results are known.
*/
public void complete(TopicIdPartition partition, Acknowledgements
acknowledgements, boolean isCommitAsync) {
- if (acknowledgements != null) {
- result.put(partition, acknowledgements);
- }
// For commitAsync, we do not wait for other results to complete,
we prepare a background event
// for every ShareAcknowledgeResponse.
// For commitAsync, we send out a background event for every
TopicIdPartition, so we use a singletonMap each time.
if (isCommitAsync) {
-
maybeSendShareAcknowledgeCommitCallbackEvent(Collections.singletonMap(partition,
acknowledgements));
+ if (acknowledgements != null) {
Review Comment:
Please could we have a test on the `ResultHandler` class directly.
##########
clients/src/test/java/org/apache/kafka/clients/consumer/internals/ShareConsumeRequestManagerTest.java:
##########
@@ -548,6 +548,39 @@ public void testAcknowledgeOnCloseWithPendingCommitSync() {
completedAcknowledgements.clear();
}
+ @Test
Review Comment:
This test seems a bit artificial and I would not add it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
