[GitHub] [kafka] gharris1727 commented on a diff in pull request #12828: KAFKA-14346: Remove hard-to-mock RestClient calls

[GitBox <gi...@apache.org>]

gharris1727 commented on code in PR #12828:
URL: https://github.com/apache/kafka/pull/12828#discussion_r1020535510


##########
connect/runtime/src/main/java/org/apache/kafka/connect/cli/ConnectDistributed.java:
##########
@@ -138,7 +141,7 @@ public Connect startConnect(Map<String, String> workerProps) {
         // herder is stopped. This is easier than having to track and own the lifecycle ourselves.
         DistributedHerder herder = new DistributedHerder(config, time, worker,
                 kafkaClusterId, statusBackingStore, configBackingStore,
-                advertisedUrl.toString(), connectorClientConfigOverridePolicy, sharedAdmin);
+                advertisedUrl.toString(), restClient, connectorClientConfigOverridePolicy, sharedAdmin, restClient);

Review Comment:
   I was trying to follow the same pattern as the sharedAdmin, but I see that the situation isn't exactly analogous. The sharedAdmin isn't directly used by the DistributedHerder, while the restClient is.
   
   Another way to think about it is that we are passing the object _without the responsibility to close it_ to the RestServer and the DistributedHerder, while we are explicitly passing the responsibility to close it as a lambda.
   
   I'm fine with either implementation, and if you think it's more natural to explicitly close it, i'll make the change.



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org