C0urante commented on code in PR #14304:
URL: https://github.com/apache/kafka/pull/14304#discussion_r1311893675
##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/ConnectorConfig.java:
##########
@@ -82,17 +86,29 @@ public class ConnectorConfig extends AbstractConfig {
public static final String KEY_CONVERTER_CLASS_CONFIG =
WorkerConfig.KEY_CONVERTER_CLASS_CONFIG;
public static final String KEY_CONVERTER_CLASS_DOC =
WorkerConfig.KEY_CONVERTER_CLASS_DOC;
public static final String KEY_CONVERTER_CLASS_DISPLAY = "Key converter
class";
+ private static final ConfigDef.Validator KEY_CONVERTER_CLASS_VALIDATOR =
ConfigDef.CompositeValidator.of(
+ ConcreteSubClassValidator.forSuperClass(Converter.class),
+ new InstantiableClassValidator()
+ );
public static final String VALUE_CONVERTER_CLASS_CONFIG =
WorkerConfig.VALUE_CONVERTER_CLASS_CONFIG;
public static final String VALUE_CONVERTER_CLASS_DOC =
WorkerConfig.VALUE_CONVERTER_CLASS_DOC;
public static final String VALUE_CONVERTER_CLASS_DISPLAY = "Value
converter class";
+ private static final ConfigDef.Validator VALUE_CONVERTER_CLASS_VALIDATOR =
ConfigDef.CompositeValidator.of(
+ ConcreteSubClassValidator.forSuperClass(Converter.class),
+ new InstantiableClassValidator()
+ );
public static final String HEADER_CONVERTER_CLASS_CONFIG =
WorkerConfig.HEADER_CONVERTER_CLASS_CONFIG;
public static final String HEADER_CONVERTER_CLASS_DOC =
WorkerConfig.HEADER_CONVERTER_CLASS_DOC;
public static final String HEADER_CONVERTER_CLASS_DISPLAY = "Header
converter class";
// The Connector config should not have a default for the header
converter, since the absence of a config property means that
// the worker config settings should be used. Thus, we set the default to
null here.
public static final String HEADER_CONVERTER_CLASS_DEFAULT = null;
+ private static final ConfigDef.Validator HEADER_CONVERTER_CLASS_VALIDATOR
= ConfigDef.CompositeValidator.of(
+ ConcreteSubClassValidator.forSuperClass(HeaderConverter.class),
+ new InstantiableClassValidator()
Review Comment:
It's not a pattern we use anywhere, but there could be a plugin type with a
non-zero-args constructor that would benefit from the
`ConcreteSubclassValidator` but not the `InstantiableClassValidator`
IMO it's also a little more readable to separate the logic into two
different places instead of having a monolithic validator, but if you think
it'd be better the other way I can combine the two into something like a
`ConnectPluginValidator`.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
