[SPARK-15526][ML][FOLLOWUP][test-maven] Make JPMML provided scope to avoid including unshaded JARs, and repromote to compile in MLlib #18637
Conversation
…mote to compile in MLlib; similar to Guava approach
|
CC @vanzin |
|
Can you add |
srowen
changed the title
|
retest this please |
vanzin
left a comment
vanzin
left a comment
There was a problem hiding this comment.
LGTM pending maven tests. If those fail you may need to mirror what core/ does with these dependencies (see copy-dependencies invocation there, and look for core/target/jars in AbstractCommandBuilder.java.
BTW those changes might be nice to have in any case, since they allow SPARK_PREPEND_CLASSES to work.
| <!-- | ||
| This profile is enabled automatically by the sbt built. It changes the scope for the guava | ||
| dependency, since we don't shade it in the artifacts generated by the sbt build. | ||
| This profile is enabled automatically by the sbt build. It changes the scope for shaded |
There was a problem hiding this comment.
I wonder if this profile is even necessary, given it does not include jetty which is also "provided" by default... but I'm too lazy to do history spelunking at the moment.
There was a problem hiding this comment.
Likewise, just updating this to update it
|
Test build #79619 has finished for PR 18637 at commit
|
|
Test build #79621 has finished for PR 18637 at commit
|
| <!-- | ||
| This profile is enabled automatically by the sbt built. It changes the scope for the guava | ||
| dependency, since we don't shade it in the artifacts generated by the sbt build. | ||
| This profile is enabled automatically by the sbt build. It changes the scope for shaded |
There was a problem hiding this comment.
Likewise, just updating this to update it
|
Test build #79646 has finished for PR 18637 at commit
|
| <groupId>org.apache.maven.plugins</groupId> | ||
| <artifactId>maven-dependency-plugin</artifactId> | ||
| <executions> | ||
| <!-- When using SPARK_PREPEND_CLASSES Spark classes compiled locally don't use |
There was a problem hiding this comment.
You need to add code in AbstractCommandBuilder.java (method buildClassPath) for this to have an actual effect.
|
Test build #79701 has finished for PR 18637 at commit
|
|
Test build #3845 has finished for PR 18637 at commit
|
|
LGTM. Merging to master. |
3 participants
What changes were proposed in this pull request?
Following the comment at https://issues.apache.org/jira/browse/SPARK-15526?focusedCommentId=16086106&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16086106 -- this change actually needed a little more work to be complete.
This also marks JPMML as
providedto make sure its JARs aren't included in thejarsoutput, but then scopes tocompileinmllib. This is how Guava is handled.How was this patch tested?
Checked result in
assembly/target/scala-2.11/jarsto verify there are no JPMML jars. Maven and SBT builds still work.