[GitHub] [kafka] kkonstantine commented on a change in pull request #10040: KAFKA-12259: Use raw config to infer connector type when returning connect status response

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

kkonstantine commented on a change in pull request #10040:
URL: https://github.com/apache/kafka/pull/10040#discussion_r569658355



##########
File path: connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractHerder.java
##########
@@ -284,7 +284,8 @@ public ConnectorStateInfo connectorStatus(String connName) {
 
         Collections.sort(taskStates);
 
-        Map<String, String> conf = config(connName);
+        final ClusterConfigState configState = configBackingStore.snapshot();

Review comment:
       I'm suspecting we might be able to skip the `snapshot` here. Since we are asking for a specific connector and since we check the status topic before, it's highly probable that the connector config is already in the `configState` that has been snapshotted in distributed or standalone herders. 
   
   Should we just change the implementations of `AbstractHerder#config` to return the raw config?

##########
File path: connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractHerder.java
##########
@@ -284,7 +284,8 @@ public ConnectorStateInfo connectorStatus(String connName) {
 
         Collections.sort(taskStates);
 
-        Map<String, String> conf = config(connName);

Review comment:
       we are removing the single use of this method. 
   
   Either we should remove it completely (including its implementations) or we could choose to extend it to return the raw config upon request. Eg. change it to `protected abstract Map<String, String> config(String connName, boolean returnRawConfig);` 
   
   `config(connName, true)` still won't have a use but maybe that's an ok exception for now. Or we can change it to `rawConfig(String connName)`. 
   
   See my comment below about whether we need to create a shallow copy of the config cluster state with the `snapshot` method. 




----------------------------------------------------------------
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.

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