JingGe commented on a change in pull request #18397:
URL: https://github.com/apache/flink/pull/18397#discussion_r789602784
##########
File path:
flink-connectors/flink-connector-kafka/src/main/java/org/apache/flink/connector/kafka/sink/KafkaSerializerWrapper.java
##########
@@ -65,8 +77,15 @@ public void open(InitializationContext context) throws
Exception {
serializerClass.getName(),
Serializer.class,
getClass().getClassLoader());
+
+ if (config.isEmpty()) {
Review comment:
I have thought about it. The question is why should user call
`configure(...)` with an empty config?
Allowing empty config going through sounds like a backdoor that the Map
reference is injected into `configure(...)`. User might want to change the
content of the map outside of the serializer/deserializer and have effect on
exist serializer/deserializer retroactively(hacking, which is not a good idea),
if the implementation of serializer/deserializer is not done correctly. The
config map as input should be used as a one-off source and should be even
turned into an immutable map before hand it over to the
`Configurable/Deserializer::configure(...)` call.
User will be aware that nothing will be done with an empty config where e.g.
content were forgot to add by mistake. How about adding logger so that any
potential issues will be trackable in the log files.
--
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: issues-unsubscr...@flink.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
