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

    https://github.com/apache/flink/pull/5200#discussion_r160598772
  
    --- Diff: 
flink-connectors/flink-connector-kafka-base/src/main/java/org/apache/flink/streaming/connectors/kafka/internals/AbstractFetcher.java
 ---
    @@ -51,7 +50,7 @@
      *            the Flink data streams.
      * @param <KPH> The type of topic/partition identifier used by Kafka in 
the specific version.
      */
    -public abstract class AbstractFetcher<T, KPH> {
    +public abstract class AbstractFetcher<T, KPH> implements 
KafkaOffsetCommitter {
    --- End diff --
    
    I agree that composition suits better here, or maybe even neither of both. 
However, the reality is that currently the offset committing logic is 
implemented tightly as part of the `AbstractFetcher`, sharing the same Kafka 
client for both fetching records and committing offsets. Decoupling that would 
require further refactoring, which I think is a bit out of scope for the 
current issue at hand.
    
    I have been thinking that we should simply have two separate service 
implementations for offset committing and record fetching. If that happens, 
then neither composition or inheritance is required; offset committing and 
record fetching simply lives as two separate services.
    
    What do you think?


---

Reply via email to