C0urante commented on code in PR #14303:
URL: https://github.com/apache/kafka/pull/14303#discussion_r1309029609
##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/ConnectorConfig.java:
##########
@@ -429,7 +429,9 @@ void enrich(ConfigDef newDef) {
final ConfigDef.Validator typeValidator =
ConfigDef.LambdaValidator.with(
(String name, Object value) -> {
validateProps(prefix);
- getConfigDefFromConfigProvidingClass(typeConfig,
(Class<?>) value);
+ if (value != null) {
Review Comment:
Ah sorry, I misread your comment!
I'm hesitant to change the behavior of instantiating a `ConnectorConfig`
unless it provides a practical, positive impact for users. It doesn't seem
significantly cleaner to move the null check either, since guarding against
null `Class<?>` instances is an inherent necessity of the current
`ConfigDef.Validator` API (and one that's taken into account with validator
classes added in https://github.com/apache/kafka/pull/14304).
I've added a comment indicating when we expect `null` values in the
validator. I'm hopeful that this should be enough to avoid adding to the
cognitive load with this change.
##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/ConnectorConfig.java:
##########
@@ -429,7 +429,9 @@ void enrich(ConfigDef newDef) {
final ConfigDef.Validator typeValidator =
ConfigDef.LambdaValidator.with(
(String name, Object value) -> {
validateProps(prefix);
- getConfigDefFromConfigProvidingClass(typeConfig,
(Class<?>) value);
+ if (value != null) {
Review Comment:
Ah sorry, I misread your comment!
I'm hesitant to change the behavior of instantiating a `ConnectorConfig`
unless it provides a practical, positive impact for users. It doesn't seem
significantly cleaner to move the null check either, since guarding against
null `Class<?>` instances is an inherent necessity of the current
`ConfigDef.Validator` API (and one that's taken into account with validator
classes added in https://github.com/apache/kafka/pull/14304).
I've added a comment indicating when we expect `null` values in the
validator. I'm hopeful that this should be enough to offset the cognitive load
added with this change.
--
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]
