C0urante commented on code in PR #12010:
URL: https://github.com/apache/kafka/pull/12010#discussion_r854092830
##########
clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java:
##########
@@ -393,10 +394,14 @@ public class ProducerConfig extends AbstractConfig {
MAX_IN_FLIGHT_REQUESTS_PER_CONNECTION_DOC)
.define(KEY_SERIALIZER_CLASS_CONFIG,
Type.CLASS,
+ ConfigDef.NO_DEFAULT_VALUE,
+ new ConfigDef.NonNullValidator(),
Review Comment:
What if someone creates a `ProducerConfig` directly, as opposed to
indirectly via a `KafkaProducer`? For example:
```java
Map<String, Object> props = new HashMap<>();
props.put("key.serializer", null);
props.put("value.serializer", null);
props.put("bootstrap.servers", "localhost:9092");
ProducerConfig producerConfig = new ProducerConfig(props);
```
This is fine right now but will break if we add this change.
I'm wondering if people might be constructing `ProducerConfig` and
`ConsumerConfig` instances in order to do some kind of validation before
storing a configuration somewhere, that's then used later to create a
producer/consumer. If so, they might be omitting the (de)serializer properties
(or rather, just using `null` as a placeholder) if the (de)serializer isn't
known at that time and they plan on using the
[constructor](https://github.com/apache/kafka/blob/301c6f44d605f7152f25057c599eeb0b01b2aef9/clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java#L288)
that accepts already-instantiated `Serializer`/`Deserializer` instances.
It's not _impossible_ that people are using this pattern but it seems much
less likely than some of the other cases that have come up here. It might be
alright to keep this. What do you think?
--
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
