[GitHub] [kafka] gharris1727 commented on a diff in pull request #13445: [KAFKA-14843] fix(connect): include base connector config to return

["gharris1727 (via GitHub)" <gi...@apache.org>]

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


##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractHerder.java:
##########
@@ -836,10 +836,19 @@ public List<ConfigKeyInfo> connectorPluginConfig(String pluginName) {
             Object plugin = p.newPlugin(pluginName);
             PluginType pluginType = PluginType.from(plugin.getClass());
             ConfigDef configDefs;
+            List<ConfigKeyInfo> results = new ArrayList<>();
             switch (pluginType) {
                 case SINK:
+                    for (ConfigDef.ConfigKey configKey : SinkConnectorConfig.configDef().configKeys().values()) {
+                        results.add(AbstractHerder.convertConfigKey(configKey));
+                    }

Review Comment:
   nit: rather than repeating this, can we extract a helper that takes `List<ConfigKeyInfo>` and `ConfigDef` and remove the `configDefs` variable here?
   
   It appears that this code made the assumption that there would only be one relevant configdef to use when generating results, which is false.



##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/AbstractHerderTest.java:
##########
@@ -984,7 +984,9 @@ private <T> void testConnectorPluginConfig(String pluginName, Supplier<T> newPlu
         assertNotNull(configs);
 
         ConfigDef expectedConfig = pluginConfig.apply(newPluginInstance.get());
-        assertEquals(expectedConfig.names().size(), configs.size());
+        int expectedConfigSize = baseConfig.map(config -> expectedConfig.names().size() + config.names().size())

Review Comment:
   nit: map baseConfig to size or else zero, to only compute expectedConfig.names().size() once.



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