C0urante commented on code in PR #14309:
URL: https://github.com/apache/kafka/pull/14309#discussion_r1586591019


##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractHerder.java:
##########
@@ -655,27 +811,38 @@ private static ConfigInfos validateClientOverrides(String 
connName,
         ConnectorClientConfigRequest connectorClientConfigRequest = new 
ConnectorClientConfigRequest(
             connName, connectorType, connectorClass, clientConfigs, 
clientType);
         List<ConfigValue> configValues = 
connectorClientConfigOverridePolicy.validate(connectorClientConfigRequest);
-        if (configValues != null) {
-            for (ConfigValue validatedConfigValue : configValues) {
-                ConfigKey configKey = 
configKeys.get(validatedConfigValue.name());
-                ConfigKeyInfo configKeyInfo = null;
-                if (configKey != null) {
-                    if (configKey.group != null) {
-                        groups.add(configKey.group);
-                    }
-                    configKeyInfo = convertConfigKey(configKey, prefix);
-                }
 
-                ConfigValue configValue = new ConfigValue(prefix + 
validatedConfigValue.name(), validatedConfigValue.value(),
-                                                          
validatedConfigValue.recommendedValues(), validatedConfigValue.errorMessages());
-                if (!configValue.errorMessages().isEmpty()) {
-                    errorCount++;
+        return prefixedConfigInfos(configDef.configKeys(), configValues, 
prefix);
+    }
+
+    private static ConfigInfos prefixedConfigInfos(Map<String, ConfigKey> 
configKeys, List<ConfigValue> configValues, String prefix) {
+        int errorCount = 0;
+        Set<String> groups = new LinkedHashSet<>();
+        List<ConfigInfo> configInfos = new ArrayList<>();
+
+        if (configValues == null) {

Review Comment:
   `ConfigDef::validate` is non-final, and plugin instances may return a 
subclass from their `config` methods that possibly returns null.
   
   I acknowledge that this is extremely unlikely, but it seems like this null 
guard is the best way to handle that scenario as opposed to, e.g., throwing an 
error and causing a 500 response to be returned. Thoughts?



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