-
Notifications
You must be signed in to change notification settings - Fork 384
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
[CELEBORN-836][BUILD] Initial support sbt #1757
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1757 +/- ##
==========================================
+ Coverage 46.72% 46.79% +0.08%
==========================================
Files 161 162 +1
Lines 10021 10046 +25
Branches 925 927 +2
==========================================
+ Hits 4681 4700 +19
- Misses 5021 5026 +5
- Partials 319 320 +1 see 5 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
@@ -0,0 +1,7 @@ | |||
[repositories] |
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.
does the file support comments? we can add comments to guide user how to use this file
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.
https://stackoverflow.com/questions/56343343/how-do-you-comment-in-the-sbt-repositories-file
The commenting style commences with the symbol #
@waitinfuture @RexXiong @FMX @AngersZhuuuu would you please take a look? note: if you encounter network issue, run the following command to apply mirror
|
|
||
usage() { | ||
cat <<EOM | ||
Usage: $script_name [options] |
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.
nit: this is not the same as sbt official options, we may want to migrate it to the official one in the future.
|
||
[repositories] | ||
local | ||
private: ${DEFAULT_ARTIFACT_REPOSITORY-file:///dev/null} |
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 the user has configured it, include the DEFAULT_ARTIFACT_REPOSITORY
in the list of repositories. cc @pan3793
the variable substitution docs: https://www.scala-sbt.org/1.x/docs/Launcher-Configuration.html#Variable+Substitution
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 variable substitution required system property instead of the environment variable, updated in 48a1aa9
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.
I see, thanks for fixing it.
I tried assembly and I got errors. Is there anything that I missed?
|
@FMX $ ./build/sbt -Pspark-3.4
sbt:celeborn> projects
[info] In file:/home/chenfu/workspace/celeborn/
[info] * celeborn
[info] celeborn-client-spark-3
[info] celeborn-client-spark-3-shaded
[info] client
[info] common
[info] master
[info] service
[info] spark-common
[info] spark-it
[info] worker
sbt:celeborn> project celeborn-client-spark-3-shaded
sbt:celeborn-client-spark-3-shaded> assembly or $ ./build/sbt -Pspark-3.4 celeborn-client-spark-3-shaded/assembly |
Thanks, merging to main |
Thanks @cfmcgrady for this PR! I tested and it really speeded up build time. Just wonder do you have a plan to add a doc describing building Celeborn? |
I plan to add the doc, once the following task is finished. cc @waitinfuture |
That's great! Thanks @cfmcgrady |
### What changes were proposed in this pull request? This PR adds packaging and testing support for Flink-related modules using SBT based on #1757 ### Why are the changes needed? improve project build speed running flink-it tests with -Pflink-1.14 ```shell sbt:celeborn> project flink-it sbt:flink-it> clean sbt:flink-it> test [success] Total time: 136 s (02:16), completed 2023-7-27 11:55:10 ``` running flink-it tests with -Pflink-1.17 ```shell $ ./build/sbt -Pflink-1.17 sbt:celeborn> project flink-it sbt:flink-it> clean sbt:flink-it> test [success] Total time: 168 s (02:48), completed 2023-7-27 11:28:35 ``` packing and shading the flink 1.14 client ```shell $ ./build/sbt -Pflink-1.14 sbt:celeborn> clean sbt:celeborn> project celeborn-client-flink-1_14-shaded sbt:celeborn-client-flink-1_14-shaded> assembly [success] Total time: 35 s, completed 2023-7-27 11:51:54 ``` packing and shading the flink 1.17 client ```shell $ ./build/sbt -Pflink-1.17 sbt:celeborn> clean sbt:celeborn> project celeborn-client-flink-1_17-shaded sbt:celeborn-client-flink-1_17-shaded> assembly [success] Total time: 39 s, completed 2023-7-27 11:49:20 ``` ### Does this PR introduce _any_ user-facing change? yes ### How was this patch tested? tested locally Closes #1764 from cfmcgrady/sbt-flink. Authored-by: Fu Chen <[email protected]> Signed-off-by: Cheng Pan <[email protected]>
What changes were proposed in this pull request?
This PR introduces the SBT build system implementation that operates independently from the current Maven build system. Different from #1627, the current implementation does not depend on
pom.xml
The implementation enables packaging and testing functionalities for server-related modules and Spark-related modules using SBT.
For Flink-related build/test, sbt build documentation, continuous integration, and plugins, they will be submitted in separate PRs
Why are the changes needed?
improve project build speed
packing the project.
packing and shading the spark 3.3 client
packing and shading the spark 2.4 client
running server-related tests
$ ./build/sbt clean test [success] Total time: 350 s (05:50), completed 2023-7-25 16:48:58
Does this PR introduce any user-facing change?
yes
How was this patch tested?
tested locally