[GitHub] [kafka] C0urante commented on a change in pull request #11369: KAFKA-13327, KAFKA-13328, KAFKA-13329: Clean up preflight connector validation

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

C0urante commented on a change in pull request #11369:
URL: https://github.com/apache/kafka/pull/11369#discussion_r718551225



##########
File path: connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractHerder.java
##########
@@ -344,12 +354,73 @@ public StatusBackingStore statusBackingStore() {
                 status.workerId(), status.trace());
     }
 
-    protected Map<String, ConfigValue> validateBasicConnectorConfig(Connector connector,
-                                                                    ConfigDef configDef,
-                                                                    Map<String, String> config) {
+    protected Map<String, ConfigValue> validateSinkConnectorConfig(ConfigDef configDef, Map<String, String> config) {
+        return SinkConnectorConfig.validate(configDef.validateAll(config), config);
+    }
+
+    protected Map<String, ConfigValue> validateSourceConnectorConfig(ConfigDef configDef, Map<String, String> config) {
         return configDef.validateAll(config);
     }
 
+    private ConfigInfos validateHeaderConverterConfig(Map<String, String> connectorConfig, ConfigValue headerConverterConfigValue) {
+        String headerConverterClass = connectorConfig.get(HEADER_CONVERTER_CLASS_CONFIG);
+
+        if (headerConverterClass == null
+            || headerConverterConfigValue == null
+            || !headerConverterConfigValue.errorMessages().isEmpty()
+        ) {
+            // Either no custom header converter was specified, or one was specified but there's a problem with it.
+            // No need to proceed any further.
+            return null;
+        }

Review comment:
       The short-circuiting pattern here where we conditionally exit the method and otherwise proceed normally doesn't seem to be playing very nicely with Checkstyle's NPath complexity metric. I kept this style because it reduces indentation and seems more readable than the alternative (where the portions of code after an `if` statement such as this one would be put inside an `else` block), but it comes at the cost that we have to add a Checkstyle suppression.




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