dajac commented on a change in pull request #11331:
URL: https://github.com/apache/kafka/pull/11331#discussion_r744103895
##########
File path:
clients/src/main/java/org/apache/kafka/clients/consumer/internals/Fetcher.java
##########
@@ -262,11 +262,12 @@ public synchronized int sendFetches() {
maxVersion = ApiKeys.FETCH.latestVersion();
}
final FetchRequest.Builder request = FetchRequest.Builder
- .forConsumer(maxVersion, this.maxWaitMs, this.minBytes,
data.toSend(), data.topicIds())
+ .forConsumer(maxVersion, this.maxWaitMs, this.minBytes,
data.toSend())
.isolationLevel(isolationLevel)
.setMaxBytes(this.maxBytes)
.metadata(data.metadata())
- .toForget(data.toForget())
+ .removed(data.toForget())
+ .replaced(data.toReplace())
Review comment:
We must be able to verify that the request sent out by this method is
correct. In the unit tests, we mock the network client for this purpose. If I
remember correctly, we can pass a request matcher to it. I need to look into
the existing unit tests for this class to see how we have done it for other
cases.
We might already have tests verifying that the version of the fetch request
sent out is correct based on wether topic ids are used or not. If we do, I
suppose that we could proceed similarly.
--
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: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
