fapaul commented on a change in pull request #18397:
URL: https://github.com/apache/flink/pull/18397#discussion_r788958657



##########
File path: 
flink-connectors/flink-connector-kafka/src/main/java/org/apache/flink/connector/kafka/source/reader/deserializer/KafkaRecordDeserializationSchema.java
##########
@@ -123,9 +121,8 @@ default void 
open(DeserializationSchema.InitializationContext context) throws Ex
      * @return A {@link KafkaRecordDeserializationSchema} that deserialize the 
value with the given
      *     deserializer.
      */
-    static <V, D extends Configurable & Deserializer<V>>
-            KafkaRecordDeserializationSchema<V> valueOnly(
-                    Class<D> valueDeserializerClass, Map<String, String> 
config) {
+    static <V, D extends Deserializer<V>> KafkaRecordDeserializationSchema<V> 
valueOnly(

Review comment:
       @lindong28 thanks for your feedback. I am wondering a bit what the best 
way here is. Removing the method seems not ideal because as you said users 
might have used it with Serializers/Deserializers implementing Configurable.
   
   In the Kafka ecosystem there seem to be different ways of configuring a 
Serializer/Deserializer one is the current one with implementing Configurable 
and the other is through the Serializer/Deserializer itself [1]. I am no Kafka 
expert so I don't know the right way to configure them. I guess we can also 
support both methods.
   
   [1] 
https://github.com/apache/kafka/blob/3dd3f3ef035501816efac23c32d517bf62fdaa56/clients/src/main/java/org/apache/kafka/common/serialization/Serializer.java#L40




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


Reply via email to