-
Notifications
You must be signed in to change notification settings - Fork 272
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
[ARCTIC-969] Refactor Logstore Source to FLINK FLIP-27 API #970
Conversation
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.
@zstraw Thanks a lot for driving this. I left some comments.
And I would suggest that the legacy logstore datastream API should be deprecated after a period of time, maybe after two versions are released, we can then remove these codes because we want to give some buffer time for those who already use these APIs, and adapt the new Source API. WDYT?
flink/v1.12/flink/src/main/java/com/netease/arctic/flink/read/source/log/LogKafkaSource.java
Outdated
Show resolved
Hide resolved
flink/v1.12/flink/src/main/java/com/netease/arctic/flink/read/source/log/LogKafkaSource.java
Outdated
Show resolved
Hide resolved
flink/v1.12/flink/src/main/java/com/netease/arctic/flink/read/source/log/LogKafkaSource.java
Outdated
Show resolved
Hide resolved
flink/v1.12/flink/src/main/java/com/netease/arctic/flink/read/source/log/LogKafkaSource.java
Outdated
Show resolved
Hide resolved
flink/v1.12/flink/src/main/java/com/netease/arctic/flink/read/source/log/LogKafkaSource.java
Show resolved
Hide resolved
flink/v1.12/flink/src/main/java/com/netease/arctic/flink/table/LogDynamicSource.java
Outdated
Show resolved
Hide resolved
flink/v1.12/flink/src/main/java/com/netease/arctic/flink/table/LogDynamicSource.java
Outdated
Show resolved
Hide resolved
flink/v1.12/flink/src/main/java/com/netease/arctic/flink/table/LogDynamicSource.java
Outdated
Show resolved
Hide resolved
flink/v1.12/flink/src/main/java/com/netease/arctic/flink/table/LogDynamicSource.java
Outdated
Show resolved
Hide resolved
flink/v1.12/flink/src/main/java/com/netease/arctic/flink/table/LogDynamicSource.java
Outdated
Show resolved
Hide resolved
694aad9
to
33af8d5
Compare
flink/v1.12/flink/src/main/java/com/netease/arctic/flink/read/source/log/LogKafkaSource.java
Outdated
Show resolved
Hide resolved
...1.12/flink/src/main/java/com/netease/arctic/flink/read/source/log/LogKafkaSourceBuilder.java
Outdated
Show resolved
Hide resolved
@zstraw Please also update the Flink datastream documentation: site/docs/ch/flink/flink-ds.md |
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.
LGTM
Why are the changes needed?
Refactor log source and sink to support Flink Flip-27 source API, which would be helpful to read file and log integrately, and Pulsar log.
Fix #482
Resolve #933
Resolve #969
Brief change log
How was this patch tested?
Add some test cases that check the changes thoroughly including negative and positive cases if possible
Add screenshots for manual tests if appropriate
Run test locally before making a pull request
Documentation