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

[ANCHOR-687] Upgrade to JDK-17 #1367

Merged

Conversation

lijamie98
Copy link
Collaborator

@lijamie98 lijamie98 commented May 16, 2024

Description

  • Upgrade JDK from 11 to 17.
  • Fix Jwt Json serialization

Context

  • JDK 11 is expected to sunset in 10/2024.

Testing

  • ./gradlew test

Documentation

Update How to Contribute doc.

Known limitations

N/A

@lijamie98 lijamie98 marked this pull request as draft May 16, 2024 16:15
@lijamie98 lijamie98 changed the title [ANCHOR-687] Upgrade to JDK-17 [DO NOT MERGE UNTIL 2.7.0] [ANCHOR-687] Upgrade to JDK-17 May 16, 2024
@lijamie98 lijamie98 changed the title [DO NOT MERGE UNTIL 2.7.0] [ANCHOR-687] Upgrade to JDK-17 [DO NOT MERGE until 2.7.0 is released] [ANCHOR-687] Upgrade to JDK-17 May 16, 2024
Copy link
Contributor

@Ifropc Ifropc left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -1,4 +1,4 @@
ARG BASE_IMAGE=gradle:7.6.4-jdk11-alpine
ARG BASE_IMAGE=gradle:7.6.4-jdk17-alpine
Copy link
Contributor

Choose a reason for hiding this comment

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

We should upgrade gradle to latest in 3.0 if we haven't already

Copy link
Collaborator Author

@lijamie98 lijamie98 May 17, 2024

Choose a reason for hiding this comment

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

This makes least impact to 2.x. We can do major upgrades in v3.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree!

if (detail instanceof Exception) {
Exception ex = (Exception) detail;

logMessageWithJson(
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this required now? Does the message get collapsed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is because logMessageWithJson(message, detail, getLogger()::error); failed serializing the message field if detail is an exception.

That's the reason that we have expand it manually.

@lijamie98 lijamie98 changed the base branch from develop to release/2.8.0 May 17, 2024 16:49
@lijamie98 lijamie98 marked this pull request as ready for review May 17, 2024 16:49
@lijamie98 lijamie98 requested a review from philipliu May 24, 2024 21:20
@lijamie98 lijamie98 changed the title [DO NOT MERGE until 2.7.0 is released] [ANCHOR-687] Upgrade to JDK-17 [ANCHOR-687] Upgrade to JDK-17 May 24, 2024
@lijamie98 lijamie98 changed the base branch from release/2.8.0 to develop May 24, 2024 21:36
@lijamie98 lijamie98 merged commit eba522b into stellar:develop May 24, 2024
7 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.

None yet

3 participants