[GitHub] [flink-connector-redis] knaufk commented on pull request #2: [FLINK-15571][connector][WIP] Redis Stream connector for Flink

[GitBox <gi...@apache.org>]

knaufk commented on PR #2:
URL: https://github.com/apache/flink-connector-redis/pull/2#issuecomment-1190766247

   Thanks @sazzad16 for you contribution and patience. I think the community would really benefit from a Redis Connector and I'll try to help you get this in. 
   
   The main challenge with the current PR is that it uses the old - about to be deprecated - source and sink APIs of Apache Flink (SourceFunction, SinkFunction). 
   
   Could you try migrating your implemention to these new APIs?
   
   For the Source, https://nightlies.apache.org/flink/flink-docs-release-1.15/docs/dev/datastream/sources/ contains a description of the interfaces and there are already a few examples like [Kafka](https://github.com/apache/flink/blob/master/flink-connectors/flink-connector-kafka/src/main/java/org/apache/flink/connector/kafka/source/KafkaSource.java) or [Pulsar](https://github.com/apache/flink/blob/master/flink-connectors/flink-connector-pulsar/src/main/java/org/apache/flink/connector/pulsar/source/PulsarSource.java). 
   
   For the source, there is [ElasticSearch](https://github.com/apache/flink-connector-elasticsearch/blob/main/flink-connector-elasticsearch-base/src/main/java/org/apache/flink/connector/elasticsearch/sink/ElasticsearchSink.java) that uses the new Sink API as well as the [FileSink](https://github.com/apache/flink/blob/master/flink-connectors/flink-connector-files/src/main/java/org/apache/flink/connector/file/sink/FileSink.java). However, I would recommend you have a look whether you can leverage the [Async Sink](https://cwiki.apache.org/confluence/display/FLINK/FLIP-171%3A+Async+Sink) like e.g. the DynamoDB Sink is doing, which is also under development right now. As prerequisite, you would need to be to asynchronously write to Redis and tell based on the resulting Future whether the request was successful or not (see Public Interfaces in the Async Sink FLIP). From what I know about Redis this should be possible and would greatly simplify the implementation of a Sink. 
   
   Lastly, I propose we split this contribution up into at least four separate PRs to get this moving more quickly.
   * the Source
   * the Sink
   * the TableSource
   * the TableSink
   
   Just start with what seems most relevant to you.
   
   I am sorry about these requests to port the implementation to the new APIs, but building on the old APIs will not be sustainable and with the new APIs we immediately get all the support for both batch and stream execution in the DataStream and Table API as well as better chances this connector ties in perfectly with checkpointing and watermarking without much work from your side. 
   
   Thanks again and looking forward to hearing from you. 
   
   
   
    
   
   


-- 
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: dev-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org