[CELEBORN-1002] Add SBT MRClientProject #1930
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1930 +/- ##
=======================================
Coverage 46.50% 46.50%
=======================================
Files 164 164
Lines 10341 10341
Branches 954 954
=======================================
Hits 4808 4808
- Misses 5218 5219 +1
+ Partials 315 314 -1 see 3 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
project/CelebornBuild.scala
Outdated
|
|
||
| (assembly / logLevel) := Level.Info, | ||
|
|
||
| // Exclude `scala-library` from assembly. |
There was a problem hiding this comment.
I think we need to include the scala-library and relocate it.
https://github.com/apache/incubator-celeborn/blob/8eba1b470e9cd54cf0cdcefc381433ce94d881a0/client-mr/mr-shaded/pom.xml#L62-L65
There was a problem hiding this comment.
It seems that this relocation does not take effect.
There was a problem hiding this comment.
then shall we remove this rule?
project/CelebornBuild.scala
Outdated
| name.startsWith("guava-") || | ||
| name.startsWith("netty-") || | ||
| name.startsWith("commons-lang3-") || | ||
| name.startsWith("RoaringBitmap-")) |
There was a problem hiding this comment.
There was a problem hiding this comment.
hmm... it seems we don't need the lz4-java dependency for MR shaded client? cc @FMX
There was a problem hiding this comment.
It may be because in the hadoop MR environment, there is no lz4-java.
There was a problem hiding this comment.
shall we also include zstd-jni? cc @FMX
There was a problem hiding this comment.
I think Hadoop has zstd-jni already. I'll make a test to verify this later.
There was a problem hiding this comment.
Looks like you have already added zstd-jni, I think it's OK.
There was a problem hiding this comment.
Hadoop uses the native zstd and does not use zstd-ini.
|
#1933 updates the shading configuration, the SBT part should be updated accordingly |
cxzl25
force-pushed
the
CELEBORN-1002
branch
from
c0e256a
to
357466f
Compare
| case m if m == "META-INF/LICENSE.txt" => MergeStrategy.discard | ||
| case m if m == "META-INF/NOTICE.txt" => MergeStrategy.discard | ||
| case m if m == "LICENSE.txt" => MergeStrategy.discard | ||
| case m if m == "NOTICE.txt" => MergeStrategy.discard |
There was a problem hiding this comment.
SBT Packaging Flink/Spark Excluding license related files will be implemented in the next PR.
cxzl25
changed the title
|
SBT and Maven have different ./build/mvn dependency:tree -Pmr -am -pl client-mr/mr
[INFO] --- maven-dependency-plugin:3.1.1:tree (default-cli) @ celeborn-client-mr_2.12 ---
[INFO] org.apache.celeborn:celeborn-client-mr_2.12:jar:0.4.0-SNAPSHOT
[INFO] +- org.apache.celeborn:celeborn-common_2.12:jar:0.4.0-SNAPSHOT:compile
[INFO] | +- com.google.guava:guava:jar:14.0.1:compile./build/sbt -Pmr celeborn-client-mr/dependencyTree | grep guava
[info] | +-com.google.guava:guava:14.0.1 (evicted by: 27.0-jre)
[info] | +-com.google.guava:guava:27.0-jre |
|
we can align the |
|
thanks, merging to main(v0.4.0). |
…jars ### What changes were proposed in this pull request? Flink/Spark jars packaged with SBT use the correct LICENSE and NOTICE. ### Why are the changes needed? #1930 (comment) ### Does this PR introduce _any_ user-facing change? ### How was this patch tested? Closes #1967 from cxzl25/CELEBORN-1031. Lead-authored-by: sychen <[email protected]> Co-authored-by: Fu Chen <[email protected]> Signed-off-by: Fu Chen <[email protected]>
…jars ### What changes were proposed in this pull request? Flink/Spark jars packaged with SBT use the correct LICENSE and NOTICE. ### Why are the changes needed? apache/celeborn#1930 (comment) ### Does this PR introduce _any_ user-facing change? ### How was this patch tested? Closes #1967 from cxzl25/CELEBORN-1031. Lead-authored-by: sychen <[email protected]> Co-authored-by: Fu Chen <[email protected]> Signed-off-by: Fu Chen <[email protected]>
What changes were proposed in this pull request?
Why are the changes needed?
Does this PR introduce any user-facing change?
How was this patch tested?