-
Notifications
You must be signed in to change notification settings - Fork 285
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 dependencies to prepare jdk17 upgrade #563
upgrade dependencies to prepare jdk17 upgrade #563
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.
Could you add a description for why the hms incompatability issue appears and some context around that?
@@ -25,4 +25,8 @@ | |||
<Bug code="RCN" /> | |||
</Match> | |||
|
|||
<Match> | |||
<Bug pattern="CT_CONSTRUCTOR_THROW"/> |
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.
Good, excluding this is the right way to go
implementation("com.google.inject.extensions:guice-persist") | ||
implementation("com.google.inject.extensions:guice-multibindings") | ||
implementation("com.google.inject.extensions:guice-servlet") | ||
implementation("joda-time:joda-time:2.10.10") |
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.
Curious why this needed to have version pinned to this
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.
If I don't add the version, I will get the following error.
Failed to resolve 'joda-time:joda-time' for project 'metacat-connector-s3'
The following dependencies are missing a version: joda-time:joda-time
When we try to run functional test under jdk 17, we get the below exception: Doing some search online: Thus we are not able to upgrade metacat to jdk17 due to hms incompatibility with java version older than 8. |
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 for adding more context, good work
Background:
We are not able to upgrade to jdk17 right now since hms is too old and is not compatible with jdk17. We will deprecate hms soon, and once it is deprecated, metacat can be upgraded to jdk17.
For this PR, we update metacat dependencies besides hms to be compatible with jdk17.
Testing plan: