Skip to content
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-33302] Allow to customize jdk version for connectors #24

Merged
merged 9 commits into from
Dec 8, 2023

Conversation

snuyanzin
Copy link

The PR allows to customize jdk versions
At the same time it still keeps 8 and 1 as default fullback option

An example of connector with this
https://github.com/snuyanzin/flink-connector-opensearch/actions/runs/6560306886/job/17817647898
where 1.16.x, 1.17.x are running with 8 and 11 and 1.18.x with 8, 11, 17

the failure of the connector is related to connector's code which still doesn't support jdk17

Copy link
Contributor

@MartijnVisser MartijnVisser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@snuyanzin Can you add a test so that it also tests a run with Java 17?

@snuyanzin
Copy link
Author

@MartijnVisser thanks for the feedback
from one side i've added a test
from the other i turned off dependency_convergence for that test. The reason is that it seems there is compress_compact dependency coming from both flink-core and test_containers however with different versions.
The interesting thing that i was able to reproduce and fix it locally however don't understand why that fix doesn't work with GHA... Anyway this seems to be a known issue and has been already fixed in downstream connectors repo like
hbase: https://github.com/snuyanzin/flink-connector-hbase/blob/e40ee4f7676659cb93bf85a5db42b98b12c091be/pom.xml#L414-L418
opensearch: https://github.com/apache/flink-connector-opensearch/blob/ab36cebc12db3aa0fa9df8a770b1845a78afe5bf/pom.xml#L275-L280
jdbc: https://github.com/apache/flink-connector-jdbc/blob/8e0496a5a9727087b38c2fc412a397a232ee0f5f/pom.xml#L297-L302

Copy link
Contributor

@MartijnVisser MartijnVisser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great improvement, thanks!

@snuyanzin snuyanzin merged commit 835ba96 into apache:ci_utils Dec 8, 2023
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants