dajac commented on a change in pull request #11331: URL: https://github.com/apache/kafka/pull/11331#discussion_r743745314
########## 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: Should we add or extend a test in `FetcherTest` to cover this change? I would like to have one which ensure that the request sent is populated correctly (especially the replaced part) by the fetcher based on the session handler. It seems that we don't have such test in the suite at the moment. -- 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