-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-40654][PROTOBUF][FOLLOW-UP] Clean up SBT build in Proto component #38240
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
LuciferYang
left a comment
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.
+1, LGTM
|
hmm... should we add for example, similar as // Exclude `scala-library` from assembly.
(assembly / assemblyPackageScala / assembleArtifact) := false,
// Exclude `pmml-model-*.jar`, `scala-collection-compat_*.jar`,
// `spark-tags_*.jar`, "guava-*.jar" and `unused-1.0.0.jar` from assembly.
(assembly / assemblyExcludedJars) := {
val cp = (assembly / fullClasspath).value
cp filter { v =>
val name = v.data.getName
name.startsWith("pmml-model-") || name.startsWith("scala-collection-compat_") ||
name.startsWith("spark-tags_") || name.startsWith("guava-") || name == "unused-1.0.0.jar"
}
},then the assembly jar just includes: |
| ), | ||
|
|
||
| (assembly / test) := false, | ||
| (assembly / test) := { }, |
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.
just for my own education, is this the same: false -> { }?
looks like a bit magic
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.
Yeah, they are same. I believe { } is much more common (googled a bit)
|
Let me get this in first in any event since it works (and the built is technically broken without this change). |
|
Merged to master. |
### What changes were proposed in this pull request? This PR cleans up the syntax, and properly copy protobuf assembly jar (currently it copies connect assembly jar by mistake). ### Why are the changes needed? For consistent code style, and correct SBT build. ### Does this PR introduce _any_ user-facing change? No, this isn't released yet. ### How was this patch tested? CI in this PR should test it out. Closes apache#38240 from HyukjinKwon/SPARK-40654. Authored-by: Hyukjin Kwon <[email protected]> Signed-off-by: Hyukjin Kwon <[email protected]>
### What changes were proposed in this pull request? This PR cleans up the syntax, and properly copy protobuf assembly jar (currently it copies connect assembly jar by mistake). ### Why are the changes needed? For consistent code style, and correct SBT build. ### Does this PR introduce _any_ user-facing change? No, this isn't released yet. ### How was this patch tested? CI in this PR should test it out. Closes apache#38240 from HyukjinKwon/SPARK-40654. Authored-by: Hyukjin Kwon <[email protected]> Signed-off-by: Hyukjin Kwon <[email protected]>
What changes were proposed in this pull request?
This PR cleans up the syntax, and properly copy protobuf assembly jar (currently it copies connect assembly jar by mistake).
Why are the changes needed?
For consistent code style, and correct SBT build.
Does this PR introduce any user-facing change?
No, this isn't released yet.
How was this patch tested?
CI in this PR should test it out.