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

refactor(java11) - convert most modules to java 11 #5836

Merged

Conversation

leifker
Copy link
Contributor

@leifker leifker commented Sep 6, 2022

This PR is one of two Java 11 options (other) - only merge one. This version converts modules to Java 11 where they are not tied to spark-lineage which requires Java 8. Unlike the other PR, there are no duplicate modules for Java 11 and Java 8, only one java version is chosen based on its usage by spark-lineage

All modules except these are converted to java 11:

li-utils
metadata-models
test-models
metadata-integration:java:datahub-client
metadata-integration:java:spark-lineage

Checklist

  • The PR conforms to DataHub's Contributing Guideline (particularly Commit Message Format)
  • Links to related issues (if applicable)
  • Tests for the changes have been added/updated (if applicable)
  • Docs related to the changes have been added/updated (if applicable). If a new feature has been added a Usage Guide has been added for the same.
  • For any breaking change/potential downtime/deprecation/big changes an entry has been made in Updating DataHub

@github-actions
Copy link

github-actions bot commented Sep 6, 2022

Unit Test Results (metadata ingestion)

       8 files  ±0         8 suites  ±0   55m 36s ⏱️ - 2m 56s
   717 tests ±0     714 ✔️ ±0  3 💤 ±0  0 ±0 
1 436 runs  ±0  1 430 ✔️ ±0  6 💤 ±0  0 ±0 

Results for commit fb4d40c. ± Comparison against base commit 325b959.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Sep 6, 2022

Unit Test Results (build & test)

584 tests  ±0   580 ✔️ ±0   12m 54s ⏱️ -1s
143 suites ±0       4 💤 ±0 
143 files   ±0       0 ±0 

Results for commit fb4d40c. ± Comparison against base commit 325b959.

♻️ This comment has been updated with latest results.

@pedro93
Copy link
Collaborator

pedro93 commented Sep 9, 2022

@pedro93 - The intent is to build any, or all, modules with jdk11. The docker images would need to updated to jdk11 as a requirement to run the jars generated with jdk11. Only certain modules would require jdk8, specifically those for supporting spark-lineage. For this later case, those specific modules can either be compile for jdk8 or for both jdk8 and jdk11 (this is the difference between the two PRs).

My question is more, what is the benefit of compiling datahub code with java 11 jdk? byte code improvements? Java 11 language features?

Following #5875 all datahub images will run in Java 11 runtime with java 8 byte code.

@leifker
Copy link
Contributor Author

leifker commented Sep 9, 2022

There are a couple benefits of compiling with jdk11 where possible.

One of them are language features for sure. For example, :metadata-integration:java:datahub-protobuf, in the project is using jdk11 and is not compatible with jdk8. I wrote that module before realizing the jdk8 dependence of the project.

I think that the most important reason is ultimately that many libraries are deprecating jdk8 support and without updating to jdk11 this locks DataHub into older versions of libraries. In turn, these older libraries may not contain important security fixes and newer functionality.

@anshbansal anshbansal added community-contribution PR or Issue raised by member(s) of DataHub Community devops PR or Issue related to DataHub backend & deployment labels Sep 10, 2022
@leifker leifker force-pushed the dleifker/default_java_11_nonexplicit branch 5 times, most recently from b1b8c95 to bc08236 Compare September 11, 2022 12:10
…and simplify ES healtcheck

spark-smoke-test - add logging
@leifker leifker force-pushed the dleifker/default_java_11_nonexplicit branch from bc08236 to 1f35ef2 Compare September 11, 2022 13:16
@shirshanka shirshanka self-assigned this Sep 13, 2022
@shirshanka shirshanka added platform PR-s that make changes to core parts of the platform and removed devops PR or Issue related to DataHub backend & deployment labels Sep 13, 2022
Copy link
Contributor

@shirshanka shirshanka left a comment

Choose a reason for hiding this comment

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

LGTM!

@shirshanka shirshanka merged commit 203a6ff into datahub-project:master Sep 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community-contribution PR or Issue raised by member(s) of DataHub Community platform PR-s that make changes to core parts of the platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants