On 4/9/24, 3:57 PM, "Dhingra, Priya" <dhipr...@amazon.com.inva <mailto:dhipr...@amazon.com.inva>LID> wrote:
CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe. Hi Ahmed and Samrat, Thanks a lot for all the feedbacks, this is my first ever contribution to apache Flink, hence I was bit unaware about few things but updated all of them as per your suggestions, thanks again for all the support here, much appreciated!! 1) I am not sure why we need to suppress warnings in the sink example in the FLIP? Removed and updated the FLIP. 2) You provided the sink example as it is the Public Interface, however the actual AsyncSink logic is mostly in the writer, so would be helpful to provide a brief of the writer or the "submitRequestEntries" Added in FLIP 3) I am not sure what customDimensions are or how are they going to be used by the client, (that's why I thought the writer example should be helpful). Removed. This is no more required, we have added in our code to support some specific usecase, no more required for apache PR. 4) Are we going to use the existing aws client providers to handle the authentication and async client creation similar to Kinesis/Firehose and DDB. I would strongly recommend we do. Yes 5) Given you suggest implementing this in "flink-connector-aws" repo, it should probably follow the same versioning of AWS connectors, hence targeting 4.3.0. Also I am not sure why we are targeting support for 1.16 given that it is out of support and 4.2 supports 1.17+. Sorry, was not aware about the versioning we should have it here. I tested the sqs sink with flint 1.16 so thought of putting the same, but was not aware about out of support. Updated now with 4.3.0 and 1.17+ 6) Are we planning to support Table API & SQL as well? This should not be much of an effort IMO so I think we should. No, not putting that in scope for first iteration. We can take that as fast follow up. 7) It would be great if we also added an implementation of Element converter given that SQS message bodies are mainly Strings if I remember correctly. We can extend it to other types for MessageAttributeValue augmentation,this should be more valuable on table API as well to use it as default Element Converter. Updated in FLIP 8. Different connectors provide different types of fault tolerant guarantees[1]. What type of fault tolerant sink guarantees flink-connector-sqs will provide ? Could you elaborate on the fault-tolerant capabilities that the flink-connector-sqs will provide? at-least-once 9) Can you help with what the minimal configuration required for instantiating the sink ? SQSSink.builder() .setSqsUrl(sqsUrl) .setSqsClientProperties(getSQSClientProperties()) .setSerializationSchema(serializationSchema) .build(); 10) Amazon SQS offers various data types [2]. Could you outline the types of SQS data the sink plans to support? SendMessageBatchRequestEntry Hi Priya, Thank you for the FLIP. sqs connector would be a great addition to the flink connector aws. +1 for all the queries raised by Ahmed. Adding to Ahmed's queries, I have a few more: 1. Different connectors provide different types of fault tolerant guarantees[1]. What type of fault tolerant sink guarantees flink-connector-sqs will provide ? Could you elaborate on the fault-tolerant capabilities that the flink-connector-sqs will provide? 2. Can you help with what the minimal configuration required for instantiating the sink ? 3. Amazon SQS offers various data types [2]. Could you outline the types of SQS data the sink plans to support? [1] https://nightlies.apache.org/flink/flink-docs-master/docs/connectors/datastream/guarantees/ <https://nightlies.apache.org/flink/flink-docs-master/docs/connectors/datastream/guarantees/> <https://nightlies.apache.org/flink/flink-docs-master/docs/connectors/datastream/guarantees/> <https://nightlies.apache.org/flink/flink-docs-master/docs/connectors/datastream/guarantees/>> [2] https://docs.aws.amazon.com/AWSSimpleQueueService/latest/APIReference/API_Types.html <https://docs.aws.amazon.com/AWSSimpleQueueService/latest/APIReference/API_Types.html> <https://docs.aws.amazon.com/AWSSimpleQueueService/latest/APIReference/API_Types.html> <https://docs.aws.amazon.com/AWSSimpleQueueService/latest/APIReference/API_Types.html>> Bests, Samrat On Sat, Apr 6, 2024 at 6:27 PM Ahmed Hamdy <hamdy10...@gmail.com <mailto:hamdy10...@gmail.com> <mailto:hamdy10...@gmail.com <mailto:hamdy10...@gmail.com>>> wrote: > Hi Dhingra, thanks for raising the FLIP > I am in favour of this addition in general. I have a couple of > comments/questions on the FLIP. > > - I am not sure why we need to suppress warnings in the sink example in the > FLIP? > - You provided the sink example as it is the Public Interface, however the > actual AsyncSink logic is mostly in the writer, so would be helpful to > provide a brief of the writer or the "submitRequestEntries" > - I am not sure what customDimensions are or how are they going to be used > by the client, (that's why I thought the writer example should be helpful). > - Are we going to use the existing aws client providers to handle the > authentication and async client creation similar to Kinesis/Firehose and > DDB. I would strongly recommend we do. > - Given you suggest implementing this in "flink-connector-aws" repo, it > should probably follow the same versioning of AWS connectors, hence > targeting 4.3.0. Also I am not sure why we are targeting support for 1.16 > given that it is out of support and 4.2 supports 1.17+. > - Are we planning to support Table API & SQL as well? This should not be > much of an effort IMO so I think we should. > - It would be great if we also added an implementation of Element converter > given that SQS message bodies are mainly Strings if I remember correctly. > We can extend it to other types for MessageAttributeValue augmentation, > this should be more valuable on table API as well to use it as default > Element Converter. > > I would love to assist with the implementation and reviews if this FLIP was > accepted. > Best Regards > Ahmed Hamdy > > > On Fri, 5 Apr 2024 at 19:19, Dhingra, Priya <dhipr...@amazon.com.inva > <mailto:dhipr...@amazon.com.inva> <mailto:dhipr...@amazon.com.inva > <mailto:dhipr...@amazon.com.inva>>lid> > wrote: > > > Hi Dev, > > > > I would like to start a discussion about FLIP-438: Amazon SQS Sink > > Connector< > > > https://cwiki.apache.org/confluence/display/FLINK/FLIP-438%3A+Amazon+SQS+Sink+Connector > > <https://cwiki.apache.org/confluence/display/FLINK/FLIP-438%3A+Amazon+SQS+Sink+Connector> > > <https://cwiki.apache.org/confluence/display/FLINK/FLIP-438%3A+Amazon+SQS+Sink+Connector> > > <https://cwiki.apache.org/confluence/display/FLINK/FLIP-438%3A+Amazon+SQS+Sink+Connector>> > >. > > This FLIP is proposing to add support for AWS SQS sink in > > flink-connector-aws repo. > > > > For more details, see FLIP-438. Looking forward to your feedback. > > > > > https://cwiki.apache.org/confluence/display/FLINK/FLIP-438%3A+Amazon+SQS+Sink+Connector > > <https://cwiki.apache.org/confluence/display/FLINK/FLIP-438%3A+Amazon+SQS+Sink+Connector> > > <https://cwiki.apache.org/confluence/display/FLINK/FLIP-438%3A+Amazon+SQS+Sink+Connector> > > <https://cwiki.apache.org/confluence/display/FLINK/FLIP-438%3A+Amazon+SQS+Sink+Connector>> > > > > Thanks, > > Priya > > > On 4/6/24, 9:07 AM, "Samrat Deb" <decordea...@gmail.com <mailto:decordea...@gmail.com> <mailto:decordea...@gmail.com <mailto:decordea...@gmail.com>>> wrote: CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe. Hi Priya, Thank you for the FLIP. sqs connector would be a great addition to the flink connector aws. +1 for all the queries raised by Ahmed. Adding to Ahmed's queries, I have a few more: 1. Different connectors provide different types of fault tolerant guarantees[1]. What type of fault tolerant sink guarantees flink-connector-sqs will provide ? Could you elaborate on the fault-tolerant capabilities that the flink-connector-sqs will provide? 2. Can you help with what the minimal configuration required for instantiating the sink ? 3. Amazon SQS offers various data types [2]. Could you outline the types of SQS data the sink plans to support? [1] https://nightlies.apache.org/flink/flink-docs-master/docs/connectors/datastream/guarantees/ <https://nightlies.apache.org/flink/flink-docs-master/docs/connectors/datastream/guarantees/> <https://nightlies.apache.org/flink/flink-docs-master/docs/connectors/datastream/guarantees/> <https://nightlies.apache.org/flink/flink-docs-master/docs/connectors/datastream/guarantees/>> [2] https://docs.aws.amazon.com/AWSSimpleQueueService/latest/APIReference/API_Types.html <https://docs.aws.amazon.com/AWSSimpleQueueService/latest/APIReference/API_Types.html> <https://docs.aws.amazon.com/AWSSimpleQueueService/latest/APIReference/API_Types.html> <https://docs.aws.amazon.com/AWSSimpleQueueService/latest/APIReference/API_Types.html>> Bests, Samrat On Sat, Apr 6, 2024 at 6:27 PM Ahmed Hamdy <hamdy10...@gmail.com <mailto:hamdy10...@gmail.com> <mailto:hamdy10...@gmail.com <mailto:hamdy10...@gmail.com>>> wrote: > Hi Dhingra, thanks for raising the FLIP > I am in favour of this addition in general. I have a couple of > comments/questions on the FLIP. > > - I am not sure why we need to suppress warnings in the sink example in the > FLIP? > - You provided the sink example as it is the Public Interface, however the > actual AsyncSink logic is mostly in the writer, so would be helpful to > provide a brief of the writer or the "submitRequestEntries" > - I am not sure what customDimensions are or how are they going to be used > by the client, (that's why I thought the writer example should be helpful). > - Are we going to use the existing aws client providers to handle the > authentication and async client creation similar to Kinesis/Firehose and > DDB. I would strongly recommend we do. > - Given you suggest implementing this in "flink-connector-aws" repo, it > should probably follow the same versioning of AWS connectors, hence > targeting 4.3.0. Also I am not sure why we are targeting support for 1.16 > given that it is out of support and 4.2 supports 1.17+. > - Are we planning to support Table API & SQL as well? This should not be > much of an effort IMO so I think we should. > - It would be great if we also added an implementation of Element converter > given that SQS message bodies are mainly Strings if I remember correctly. > We can extend it to other types for MessageAttributeValue augmentation, > this should be more valuable on table API as well to use it as default > Element Converter. > > I would love to assist with the implementation and reviews if this FLIP was > accepted. > Best Regards > Ahmed Hamdy > > > On Fri, 5 Apr 2024 at 19:19, Dhingra, Priya <dhipr...@amazon.com.inva > <mailto:dhipr...@amazon.com.inva> <mailto:dhipr...@amazon.com.inva > <mailto:dhipr...@amazon.com.inva>>lid> > wrote: > > > Hi Dev, > > > > I would like to start a discussion about FLIP-438: Amazon SQS Sink > > Connector< > > > https://cwiki.apache.org/confluence/display/FLINK/FLIP-438%3A+Amazon+SQS+Sink+Connector > > <https://cwiki.apache.org/confluence/display/FLINK/FLIP-438%3A+Amazon+SQS+Sink+Connector> > > <https://cwiki.apache.org/confluence/display/FLINK/FLIP-438%3A+Amazon+SQS+Sink+Connector> > > <https://cwiki.apache.org/confluence/display/FLINK/FLIP-438%3A+Amazon+SQS+Sink+Connector>> > >. > > This FLIP is proposing to add support for AWS SQS sink in > > flink-connector-aws repo. > > > > For more details, see FLIP-438. Looking forward to your feedback. > > > > > https://cwiki.apache.org/confluence/display/FLINK/FLIP-438%3A+Amazon+SQS+Sink+Connector > > <https://cwiki.apache.org/confluence/display/FLINK/FLIP-438%3A+Amazon+SQS+Sink+Connector> > > <https://cwiki.apache.org/confluence/display/FLINK/FLIP-438%3A+Amazon+SQS+Sink+Connector> > > <https://cwiki.apache.org/confluence/display/FLINK/FLIP-438%3A+Amazon+SQS+Sink+Connector>> > > > > Thanks, > > Priya > > >
