-
Notifications
You must be signed in to change notification settings - Fork 85
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
[FLINK-34113] Update flink-connector-elasticsearch to be compatible with updated SinkV2 interfaces #88
Conversation
Thanks for opening this pull request! Please check out our contributing guidelines. (https://flink.apache.org/contributing/how-to-contribute.html) |
c8da2cc
to
0b7eabb
Compare
pom.xml
Outdated
<groupId>org.yaml</groupId> | ||
<artifactId>snakeyaml</artifactId> | ||
<version>1.31</version> | ||
</dependency> |
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.
CI passes locally https://github.com/Jiabao-Sun/flink-connector-elasticsearch/actions/runs/7665812325/job/20892471652. |
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.
@Jiabao-Sun Thanks for this, but then we also need to make sure that this is tested against 1.19-SNAPSHOT if I'm not mistaken. Can you add that to this PR, so we can see if everything will compile against 1.17, 1.18 and 1.19, or what we need to drop?
Hey @MartijnVisser, this PR can be tested against 1.19-SNAPSHOT as well. |
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. Only left a single comment.
pom.xml
Outdated
@@ -339,6 +339,13 @@ under the License. | |||
<version>2.1</version> | |||
</dependency> | |||
|
|||
<!-- For dependency convergence --> |
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.
This change should be placed to a seperate commit.
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 @WencongLiu for the review.
I have split it into a hotfix commit.
Please help look it again.
f18673b
to
43cf63c
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 @Jiabao-Sun for the update, LGTM
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.
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. Thanks a lot @Jiabao-Sun !
[FLINK-34113] Update flink-connector-elasticsearch to be compatible with updated SinkV2 interfaces
We can use
TestSinkInitContext
instead ofMockInitContext
to resolve this compilation problem.