-
Notifications
You must be signed in to change notification settings - Fork 63
[FLINK-36971][Connectors/Sqs] Add Sqs SQL connector #187
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
009002c
to
d5178f0
Compare
@hlteoh37 could you please help review this PR? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you Ahmed for raising the PR!
Can you elaborate the detail (if any) of manual tests have been done with the SQS SQL connector?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR, left a clarification question
jar, | ||
Arrays.asList( | ||
"org/apache/flink/", | ||
"org/apache/commons/", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we shade and relocate this to org.apache.flink.connector.sqs.shaded
like the other dependencies?
<include>org.apache.flink:flink-connector-sqs</include> | ||
<include>software.amazon.awssdk:*</include> | ||
<include>org.reactivestreams:*</include> | ||
<include>com.typesafe.netty:*</include> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we double check if com.typesafe.netty:*
is actually bundled and relocated in the jar please? I did a quick check and couldn't find it
Purpose of the change
Add SQS SQL connector wrapper for table api connector
Verifying this change
Significant changes
(Please check any boxes [x] if the answer is "yes". You can first publish the PR and check them afterwards, for convenience.)
@Public(Evolving)
)