mjsax commented on code in PR #17021: URL: https://github.com/apache/kafka/pull/17021#discussion_r1805395971
########## clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java: ########## @@ -556,11 +561,12 @@ static KafkaAdminClient createInternal( apiVersions, time, 1, - (int) TimeUnit.HOURS.toMillis(1), + (int) TimeUnit.HOURS.toMillis(1), null, metadataManager.updater(), - (hostResolver == null) ? new DefaultHostResolver() : hostResolver); + (hostResolver == null) ? new DefaultHostResolver() : hostResolver, null, Review Comment: as above ########## clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java: ########## @@ -664,6 +674,8 @@ public void close(Duration timeout) { long now = time.milliseconds(); long newHardShutdownTimeMs = now + waitTimeMs; long prev = INVALID_SHUTDOWN_TIME; + clientTelemetryReporter.ifPresent(ClientTelemetryReporter::initiateClose); + metrics.close(); Review Comment: Just double checking: could either `initiateClose` or `metrics.close()` throw? If yes, would it be a problem that we don't execute the code below? ########## clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java: ########## @@ -556,11 +561,12 @@ static KafkaAdminClient createInternal( apiVersions, time, 1, - (int) TimeUnit.HOURS.toMillis(1), + (int) TimeUnit.HOURS.toMillis(1), null, Review Comment: nit: `null` should go into it's own line ########## clients/src/main/java/org/apache/kafka/clients/admin/Admin.java: ########## @@ -1844,7 +1844,7 @@ default ListShareGroupsResult listShareGroups() { * Metrics not matching these types are silently ignored. * Executing this method for a previously registered metric is a benign operation and results in updating that metrics entry. * - * @param metric The application metric to register + * @param metric, the application metric to register Review Comment: Seems like a rebase error? Original code looks right -- 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