-
Notifications
You must be signed in to change notification settings - Fork 15
[FLINK-21373] Add RabbitMQ SinkV2 Implementation, Port Flink version to Flink 1.19 #29
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
Thanks for opening this pull request! Please check out our contributing guidelines. (https://flink.apache.org/contributing/how-to-contribute.html) |
b519ef2
to
0b3fd4b
Compare
@MartijnVisser @zentol could you please review? |
0b3fd4b
to
ea4f605
Compare
ea4f605
to
cdb6c0d
Compare
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 @vahmed-hamdy, great work!
Added some suggestions
...k-connector-rabbitmq/src/main/java/org/apache/flink/connector/rabbitmq/common/Constants.java
Show resolved
Hide resolved
...bitmq/src/main/java/org/apache/flink/connector/rabbitmq/common/RabbitMQConnectionConfig.java
Show resolved
Hide resolved
...bitmq/src/main/java/org/apache/flink/connector/rabbitmq/common/RabbitMQConnectionConfig.java
Show resolved
Hide resolved
...bitmq/src/main/java/org/apache/flink/connector/rabbitmq/common/RabbitMQConnectionConfig.java
Outdated
Show resolved
Hide resolved
...c/main/java/org/apache/flink/connector/rabbitmq/common/RabbitMQConnectionSetupException.java
Show resolved
Hide resolved
...ctor-rabbitmq/src/main/java/org/apache/flink/connector/rabbitmq/sink/RabbitMQSinkWriter.java
Outdated
Show resolved
Hide resolved
...ctor-rabbitmq/src/main/java/org/apache/flink/connector/rabbitmq/sink/RabbitMQSinkWriter.java
Show resolved
Hide resolved
...ctor-rabbitmq/src/main/java/org/apache/flink/connector/rabbitmq/sink/RabbitMQSinkWriter.java
Outdated
Show resolved
Hide resolved
...ctor-rabbitmq/src/main/java/org/apache/flink/connector/rabbitmq/sink/RabbitMQSinkWriter.java
Show resolved
Hide resolved
...connector-rabbitmq/src/main/java/org/apache/flink/streaming/connectors/rabbitmq/RMQSink.java
Show resolved
Hide resolved
cdb6c0d
to
434558b
Compare
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 @vahmed-hamdy, looks good 👍
...ctor-rabbitmq/src/main/java/org/apache/flink/connector/rabbitmq/sink/RabbitMQSinkWriter.java
Show resolved
Hide resolved
Any updates? |
@paulh86 Blocked on review from the community, will reach out on dev slack channel for a reviewer |
Hello, What is the status of this PR? Thanks |
@rjbaucells Hello, It is pending approval from @MartijnVisser |
Hi, are there any plans to merge this PR. Since, the SourceFunction is deprecated in the FLINK and is being replaced with SOURCE API this PR is having the latest code to migrate the connector to use Source and Sink API. |
Any further update? From what I understand, the 1.17 version of the Flink connector does not work with 1.18, 1.19 or 1.20 so anybody wanting to connect Flink to Rabbit is stuck on 1.17. According to the Flink docs anyway here https://nightlies.apache.org/flink/flink-docs-release-1.18/docs/connectors/datastream/rabbitmq/ |
@@ -48,7 +48,7 @@ under the License. | |||
</modules> | |||
|
|||
<properties> | |||
<flink.version>1.16.0</flink.version> | |||
<flink.version>1.19.0</flink.version> |
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.
I see that we have release 3.0.1-1.17 & 1.16 in maven repo, This PR will bump the version to 1.19. What is our release plan for 1.18 and will we skip this version
Are there any plans to merge this PR? Seem like its there just one open question about 1.18 (and now 1.20 as well...). 1.19 is critical for python support as well. Python 3.11 requires flink 1.19 or greater. |
Is this still waiting on @MartijnVisser ? |
Too bad, this never got merged :-( Completely breaks our desire to use flink for an rabbitmq to kafka usecase |
Unfortunately I consider Rabbit to Flink support now deprecated and should be taken off the flink website as “supported”. Martijn should hand over merge permissions if he is the only person who has them. He was contributing to confluent repros late last year so I assume he is still available? |
Why is not merged this PR? |
This isn't correct: it's pending approval by any Apache Flink committer, not specifically me. I don't have the bandwidth nor the knowledge about RabbitMQ to perform a proper review. |
@MartijnVisser yup sorry it was just an assumption from your activity on the repo I guess, would you please help nominate someone from the committers? |
@dannycranmer I saw your commits on this repo, by any chance if you can review the changes and merge the PR ? any plans for rabbitmq support with Flink 2.0 ? |
Purpose of the change
Add SinkV2 implemetation for RabbitMQ.
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)
)