Github user tzulitai commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5958#discussion_r186338721
  
    --- Diff: 
flink-core/src/main/java/org/apache/flink/api/common/serialization/DeserializationSchema.java
 ---
    @@ -42,14 +42,22 @@
     @Public
     public interface DeserializationSchema<T> extends Serializable, 
ResultTypeQueryable<T> {
     
    +   /**
    +    * @deprecated Use {@link #deserialize(ConsumerRecordMetaInfo)} .
    +    */
    +   @Deprecated
    +   T deserialize(byte[] message) throws IOException;
    +
        /**
         * Deserializes the byte message.
         *
    -    * @param message The message, as a byte array.
    +    * @param consumerRecordMetaInfossage The message, as a {@link 
ConsumerRecordMetaInfo}.
         *
         * @return The deserialized message as an object (null if the message 
cannot be deserialized).
         */
    -   T deserialize(byte[] message) throws IOException;
    +   default T deserialize(ConsumerRecordMetaInfo 
consumerRecordMetaInfossage) throws IOException {
    --- End diff --
    
    I'm actually not sure that we should continue using this class, for the 
following reasons:
    
    1. The class is actually placed under a non-ideal package:
    `o.a.f.api.common.serialization`, whereas is should be placed under some 
`o.a.f.connectors.kafka....`.
    The reason it is currently placed under this package was because the 
`DeserializationSchema` was initially intended to be commonly used by all 
connectors. However, over time, things have proven that each connector will 
benefit from their own version of a schema class.
    
    So, it actually might make sense to deprecate the whole 
`DeserializationSchema` class now, and have a new class (maybe called 
`KafkaDeserializationSchema` / `KafkaSerializationSchema`) under a correct 
Kafka package.
    
    What do you think?


---

Reply via email to