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

Reply via email to