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/>
[2]
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>> 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>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>
> >.
> > 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>
> >
> > Thanks,
> > Priya
> >
> 
 
 

On 4/6/24, 9:07 AM, "Samrat Deb" <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/>
[2]
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>> 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>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>
> >.
> > 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>
> >
> > Thanks,
> > Priya
> >
>



Reply via email to