-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-53138][CORE][BUILD] Split common-utils Java code into a new module #51868
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
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.
not sure if the style is right here
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.
common/variant still depends on common/utils(Scala) because this is a Scala class
common/network-yarn/pom.xml
Outdated
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.
this does not change the output of artifacts, but is required to fix the scaladocs packaging
[INFO] --- scala:4.9.5:doc-jar (attach-scaladocs) @ spark-network-yarn_2.13 ---
scaladoc error: fatal error: object scala in compiler mirror not found.
|
this is ready for review, cc @LuciferYang @mridulm @dongjoon-hyun |
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.
Thank you for this approach, @pan3793 .
.github/workflows/build_and_test.yml
Outdated
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.
Similar changes should be incorporated into the maven_test.yml file. Meanwhile, both of these files should be utilized for the daily tests of branch-4.0/3.5. It is necessary to consider compatibility issues arising from the absence of certain modules in the relevant branches.
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.
Seems branch-3.5 does not have the Maven daily test, I changed maven_test.yml in 21c892f99bcac44b28038ab1a0fd1034d09d3bc4, please help check.
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.
The cost of rewriting the Scala code in common-utils into Java would be significant. If we want to address this issue but have no better alternative, then I accept this solution.
But let's wait for others' opinions.
|
Rebased to resolve conflicts |
|
Rebased to resolve conflicts |
|
If there are no objections within 24 hours, I will merge this one |
|
Merged into master for Apache Spark 4.1.0. Thanks @pan3793 and @dongjoon-hyun |
What changes were proposed in this pull request?
This PR splits the Java code of
common/utilsinto a new modulecommon/utils-java, except for:common/utils/src/main/java/org/apache/spark/storage/StorageLevelMapper.javacommon/utils/src/main/java/org/apache/spark/SparkThrowable.javaA few utility methods are rewritten in Java to avoid depending on
scala-library.Why are the changes needed?
To make YARN ESS (
common/network-yarn) scala-free again.Read the discussion of SPARK-52942 (#51650) for more details and the following PR.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Now, YARN ESS jar is scala-free, rocksdbjni and netty contribute the major size.
Was this patch authored or co-authored using generative AI tooling?
No.