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

Upgrade to Java 17 and use getPlatformClassLoader for jar parameter #118

Merged
merged 1 commit into from
Jul 26, 2023
Merged

Upgrade to Java 17 and use getPlatformClassLoader for jar parameter #118

merged 1 commit into from
Jul 26, 2023

Conversation

ebyhr
Copy link
Member

@ebyhr ebyhr commented Jul 26, 2023

trinodb/trino#17667 is trying to download jar from maven programatically and use jar parameter, but JdbcUtils.class.getClassLoader() caused class loader issue.

@cla-bot cla-bot bot added the cla-signed label Jul 26, 2023
@ebyhr ebyhr changed the title Upgrade to Java 11 and use getSystemClassLoader for jar parameter Upgrade to Java 11 and use getPlatformClassLoader for jar parameter Jul 26, 2023
@ebyhr ebyhr requested review from wendigo and kokosing July 26, 2023 07:41
@wendigo
Copy link
Contributor

wendigo commented Jul 26, 2023

Why not move to 17?

@ebyhr ebyhr changed the title Upgrade to Java 11 and use getPlatformClassLoader for jar parameter Upgrade to Java 17 and use getPlatformClassLoader for jar parameter Jul 26, 2023
@wendigo
Copy link
Contributor

wendigo commented Jul 26, 2023

@ebyhr the class loader change is gone and commit message is out of sync now

@ebyhr
Copy link
Member Author

ebyhr commented Jul 26, 2023

@wendigo CI is green now.

@wendigo
Copy link
Contributor

wendigo commented Jul 26, 2023

@ebyhr please extract docker bump to a separate commit. other than that LGTM

Copy link
Contributor

@wendigo wendigo left a comment

Choose a reason for hiding this comment

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

LGTM % commit boundaries

@ebyhr
Copy link
Member Author

ebyhr commented Jul 26, 2023

Docker bump is directly related to the change because CI doesn't pass with centos7-oj11. I will merge this PR as-is.

@ebyhr ebyhr merged commit 7604fb8 into trinodb:master Jul 26, 2023
@ebyhr ebyhr deleted the ebi/jdbc-utils branch July 26, 2023 11:35
@wendigo
Copy link
Contributor

wendigo commented Jul 26, 2023

@ebyhr fine, I wasn't aware :)

@wendigo
Copy link
Contributor

wendigo commented Jul 26, 2023

Oh, right, higher java version!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants