-
Notifications
You must be signed in to change notification settings - Fork 29k
SPARK-2798 [BUILD] Correct several small errors in Flume module pom.xml files #1726
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
|
Cool, 25cad6a fixes the core flume-sink test issue. Still worth a thought about these other touch-ups here, especially the avro version. |
|
QA tests have started for PR 1726. This patch merges cleanly. |
|
QA results for PR 1726: |
|
QA tests have started for PR 1726. This patch merges cleanly. |
|
QA results for PR 1726: |
|
Jenkins, retest this please. |
|
QA tests have started for PR 1726. This patch merges cleanly. |
|
QA results for PR 1726: |
|
@pwendell I think this is a good set of changes. Can you take a quick look. |
|
I tested, but compile failed. |
external/flume-sink/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.
Add the dependency. So make-distribution.sh can work.
<dependency>
<groupId>org.apache.spark</groupId>
<artifactId>spark-streaming_${scala.binary.version}</artifactId>
<version>${project.version}</version>
<scope>test</scope>
</dependency>|
@witgo Yes I see the same issue now after a rebase. I added your extra dependency, rebased, and it works. Let's see what Jenkins says. |
|
QA tests have started for PR 1726 at commit
|
|
QA tests have finished for PR 1726 at commit
|
external/flume-sink/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.
Why is this being removed? This is being explicitly mentioned since Flume is Java code - so the scala-library needs to be deployed.
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.
It's already brought in by core. I don't think this is harmful, but just a bit nonstandard. Except for streaming, the other modules don't seem to do this.
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 spark sink does not depend on core, so core won't pull this in.
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.
Through spark-streaming then? take a look at mvn dependency:tree -pl external/flume-sink/; it is included. It is not a big deal now that the dependency here is not on a fixed version, but inherits from the parent. But it seems that most modules do not declare this themselves.
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 sink does not depend on streaming either. The streaming jar should not even be dependency - even for test only the streaming test jar is required:
https://gist.github.com/harishreedharan/c07f27a39ff76b001ff7
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.
Isn't that the problem? You can see the dependency on streaming test code,
but not streaming non-test code. But non-test streaming code is required at
runtime. Who are we to argue with the compiler?
On Sat, Aug 23, 2014 at 12:47 AM, Hari Shreedharan <[email protected]
wrote:
In external/flume-sink/pom.xml:
@@ -65,12 +66,15 @@
<groupId>org.scala-lang</groupId><artifactId>scala-library</artifactId>The sink does not depend on streaming either. The streaming jar should not
even be dependency - even for test only the streaming test jar is required:[INFO] org.apache.spark:spark-streaming-flume-sink_2.10:jar:1.1.0-SNAPSHOT
[INFO] +- org.apache.flume:flume-ng-sdk:jar:1.4.0:compile
[INFO] | +- org.apache.avro:avro:jar:1.7.6:compile
[INFO] | | +- org.codehaus.jackson:jackson-core-asl:jar:1.9.13:compile
[INFO] | | +- org.codehaus.jackson:jackson-mapper-asl:jar:1.8.8:compile
[INFO] | | +- com.thoughtworks.paranamer:paranamer:jar:2.3:compile
[INFO] | | +- org.xerial.snappy:snappy-java:jar:1.1.1.3:compile
[INFO] | | - org.apache.commons:commons-compress:jar:1.4.1:compile
[INFO] | | - org.tukaani:xz:jar:1.0:compile
[INFO] | - org.apache.avro:avro-ipc:jar:1.7.6:compile
[INFO] +- org.apache.flume:flume-ng-core:jar:1.4.0:compile
[INFO] | +- org.apache.flume:flume-ng-configuration:jar:1.4.0:compile
[INFO] | +- org.slf4j:slf4j-api:jar:1.7.5:compile
[INFO] | +- com.google.guava:guava:jar:14.0.1:provided
[INFO] | +- commons-io:commons-io:jar:2.1:compile
[INFO] | +- commons-codec:commons-codec:jar:1.5:compile
[INFO] | +- log4j:log4j:jar:1.2.17:compile
[INFO] | +- org.slf4j:slf4j-log4j12:jar:1.7.5:compile
[INFO] | +- commons-cli:commons-cli:jar:1.2:compile
[INFO] | +- commons-lang:commons-lang:jar:2.5:compile
[INFO] | +- joda-time:joda-time:jar:2.1:compile
[INFO] | +- org.mortbay.jetty:servlet-api:jar:2.5-20110124:compile
[INFO] | +- org.mortbay.jetty:jetty-util:jar:6.1.26:compile
[INFO] | +- org.mortbay.jetty:jetty:jar:6.1.26:compile
[INFO] | +- com.google.code.gson:gson:jar:2.2.2:compile
[INFO] | - org.apache.mina:mina-core:jar:2.0.4:compile
[INFO] +- org.scala-lang:scala-library:jar:2.10.4:compile
[INFO] +- org.scalatest:scalatest_2.10:jar:2.1.5:test
[INFO] | - org.scala-lang:scala-reflect:jar:2.10.4:test
[INFO] -
org.apache.spark:spark-streaming_2.10:test-jar:tests:1.1.0-SNAPSHOT:test
[INFO] +- org.apache.spark:spark-core_2.10:jar:1.1.0-SNAPSHOT:test
[INFO] | +- org.apache.hadoop:hadoop-client:jar:1.0.4:test
[INFO] | | - org.apache.hadoop:hadoop-core:jar:1.0.4:test
[INFO] | | +- xmlenc:xmlenc:jar:0.52:test
[INFO] | | +- org.apache.commons:commons-math:jar:2.1:test
[INFO] | | +- commons-configuration:commons-configuration:jar:1.6:test
[INFO] | | | +- commons-collections:commons-collections:jar:3.2.1:test
[INFO] | | | +- commons-digester:commons-digester:jar:1.8:test
[INFO] | | | | - commons-beanutils:commons-beanutils:jar:1.7.0:test
[INFO] | | | - commons-beanutils:commons-beanutils-core:jar:1.8.0:test
[INFO] | | +- commons-el:commons-el:jar:1.0:test
[INFO] | | +- hsqldb:hsqldb:jar:1.8.0.10:test
[INFO] | | - oro:oro:jar:2.0.8:test
[INFO] | +- net.java.dev.jets3t:jets3t:jar:0.7.1:test
[INFO] | | - commons-httpclient:commons-httpclient:jar:3.1:test
[INFO] | +- org.apache.curator:curator-recipes:jar:2.4.0:test
[INFO] | | +- org.apache.curator:curator-framework:jar:2.4.0:test
[INFO] | | | - org.apache.curator:curator-client:jar:2.4.0:test
[INFO] | | - org.apache.zookeeper:zookeeper:jar:3.4.5:test
[INFO] | | - jline:jline:jar:0.9.94:test
[INFO] | +- org.eclipse.jetty:jetty-plus:jar:8.1.14.v20131031:test
[INFO] | | +-
org.eclipse.jetty.orbit:javax.transaction:jar:1.1.1.v201105210645:test
[INFO] | | +- org.eclipse.jetty:jetty-webapp:jar:8.1.14.v20131031:test
[INFO] | | | +- org.eclipse.jetty:jetty-xml:jar:8.1.14.v20131031:test
[INFO] | | | - org.eclipse.jetty:jetty-servlet:jar:8.1.14.v20131031:test
[INFO] | | - org.eclipse.jetty:jetty-jndi:jar:8.1.14.v20131031:test
[INFO] | | -
org.eclipse.jetty.orbit:javax.mail.glassfish:jar:1.4.1.v201005082020:test
[INFO] | | -
org.eclipse.jetty.orbit:javax.activation:jar:1.1.0.v201105071233:test
[INFO] | +- org.eclipse.jetty:jetty-security:jar:8.1.14.v20131031:test
[INFO] | +- org.eclipse.jetty:jetty-util:jar:8.1.14.v20131031:test
[INFO] | +- org.apache.commons:commons-lang3:jar:3.3.2:test
[INFO] | +- com.google.code.findbugs:jsr305:jar:1.3.9:test
[INFO] | +- org.slf4j:jul-to-slf4j:jar:1.7.5:test
[INFO] | +- org.slf4j:jcl-over-slf4j:jar:1.7.5:test
[INFO] | +- com.ning:compress-lzf:jar:1.0.0:test
[INFO] | +- net.jpountz.lz4:lz4:jar:1.2.0:test
[INFO] | +- com.twitter:chill_2.10:jar:0.3.6:test
[INFO] | | - com.esotericsoftware.kryo:kryo:jar:2.21:test
[INFO] | | +-
com.esotericsoftware.reflectasm:reflectasm:jar:shaded:1.07:test
[INFO] | | +- com.esotericsoftware.minlog:minlog:jar:1.2:test
[INFO] | | - org.objenesis:objenesis:jar:1.2:test
[INFO] | +- com.twitter:chill-java:jar:0.3.6:test
[INFO] | +- commons-net:commons-net:jar:2.2:test
[INFO] | +-
org.spark-project.akka:akka-remote_2.10:jar:2.2.3-shaded-protobuf:test
[INFO] | | +-
org.spark-project.akka:akka-actor_2.10:jar:2.2.3-shaded-protobuf:test
[INFO] | | | - com.typesafe:config:jar:1.0.2:test
[INFO] | | +- io.netty:netty:jar:3.6.6.Final:test
[INFO] | | +-
org.spark-project.protobuf:protobuf-java:jar:2.4.1-shaded:test
[INFO] | | - org.uncommons.maths:uncommons-maths:jar:1.2.2a:test
[INFO] | +-
org.spark-project.akka:akka-slf4j_2.10:jar:2.2.3-shaded-protobuf:test
[INFO] | +- org.json4s:json4s-jackson_2.10:jar:3.2.10:test
[INFO] | | +- org.json4s:json4s-core_2.10:jar:3.2.10:test
[INFO] | | | +- org.json4s:json4s-ast_2.10:jar:3.2.10:test
[INFO] | | | - org.scala-lang:scalap:jar:2.10.4:test
[INFO] | | | - org.scala-lang:scala-compiler:jar:2.10.4:test
[INFO] | | - com.fasterxml.jackson.core:jackson-databind:jar:2.3.1:test
[INFO] | | +- com.fasterxml.jackson.core:jackson-annotations:jar:2.3.0:test
[INFO] | | - com.fasterxml.jackson.core:jackson-core:jar:2.3.1:test
[INFO] | +- colt:colt:jar:1.2.0:test
[INFO] | | - concurrent:concurrent:jar:1.3.4:test
[INFO] | +- org.apache.mesos:mesos:jar:shaded-protobuf:0.18.1:test
[INFO] | +- io.netty:netty-all:jar:4.0.23.Final:test
[INFO] | +- com.clearspring.analytics:stream:jar:2.7.0:test
[INFO] | +- com.codahale.metrics:metrics-core:jar:3.0.0:test
[INFO] | +- com.codahale.metrics:metrics-jvm:jar:3.0.0:test
[INFO] | +- com.codahale.metrics:metrics-json:jar:3.0.0:test
[INFO] | +- com.codahale.metrics:metrics-graphite:jar:3.0.0:test
[INFO] | +- org.tachyonproject:tachyon-client:jar:0.5.0:test
[INFO] | | - org.tachyonproject:tachyon:jar:0.5.0:test
[INFO] | +- org.spark-project:pyrolite:jar:2.0.1:test
[INFO] | - net.sf.py4j:py4j:jar:0.8.2.1:test
[INFO] - org.eclipse.jetty:jetty-server:jar:8.1.14.v20131031:test
[INFO] +-
org.eclipse.jetty.orbit:javax.servlet:jar:3.0.0.v201112011016:test
[INFO] +- org.eclipse.jetty:jetty-continuation:jar:8.1.14.v20131031:test
[INFO] - org.eclipse.jetty:jetty-http:jar:8.1.14.v20131031:test
[INFO] - org.eclipse.jetty:jetty-io:jar:8.1.14.v20131031:test—
Reply to this email directly or view it on GitHub
https://github.com/apache/spark/pull/1726/files#r16626588.
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.
Look at line 30 onwards in the gist. It lists core and a whole lot of others as deps, not streaming main code as dependency for the streaming-test code. That is what I am talking about.
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 like an open maven bug: https://jira.codehaus.org/browse/MNG-1378
Does not seem like there is scope for resolution on that one.
|
Actually, the Spark Sink test code only depends on the test jar. The test jar seems to depend on the main jar. I am not sure why the sink's pom must also pull in the main jar even though it does not depend on it directly. |
|
How do you explain the compile failure then? The test code does plainly On Sat, Aug 23, 2014 at 12:43 AM, Hari Shreedharan <[email protected]
|
|
Yes, the test code depends on test code from streaming - not on non-test code (directly - the test code may depend on non-test code - isn't it maven's job to figure out transitive deps?). If test dependencies do not bring in transitive dependencies that is a bug in maven no? How is a user supposed to list all the dependencies of his test dependencies in his own pom? |
|
Looks like there is no real way to have the test jar depend on the non-test jar. The only way to do this is to do the "preferred way" in this page: http://maven.apache.org/plugins/maven-jar-plugin/examples/create-test-jar.html that requires a ton of work |
|
Jenkins, test this please. |
|
@pwendell @harishreedharan @srowen There is a better fix for fixing the maven build alone. There is no reason to depend on spark-streaming tests. So I eliminated that dependency completely and submitted a PR |
…rect Scala dependency, fix '2.10' in Flume dependency
|
QA tests have started for PR 1726 at commit
|
|
QA tests have finished for PR 1726 at commit
|
|
LGTM. Also spoke to @pwendell offline that its cool to add flume version (that is version of libraries used in external projects) in root pom.xml. Test this once more and merging this. |
|
Jenkins, test this please. |
|
QA tests have started for PR 1726 at commit
|
|
QA tests have finished for PR 1726 at commit
|
|
Jenkins, test this please. |
|
QA tests have started for PR 1726 at commit
|
|
QA tests have finished for PR 1726 at commit
|
…ml files (EDIT) Since the scalatest issue was since resolved, this is now about a few small problems in the Flume Sink `pom.xml` - `scalatest` is not declared as a test-scope dependency - Its Avro version doesn't match the rest of the build - Its Flume version is not synced with the other Flume module - The other Flume module declares its dependency on Flume Sink slightly incorrectly, hard-coding the Scala 2.10 version - It depends on Scala Lang directly, which it shouldn't Author: Sean Owen <[email protected]> Closes #1726 from srowen/SPARK-2798 and squashes the following commits: a46e2c6 [Sean Owen] scalatest to test scope, harmonize Avro and Flume versions, remove direct Scala dependency, fix '2.10' in Flume dependency (cherry picked from commit cd30db5) Signed-off-by: Tathagata Das <[email protected]>
…ml files (EDIT) Since the scalatest issue was since resolved, this is now about a few small problems in the Flume Sink `pom.xml` - `scalatest` is not declared as a test-scope dependency - Its Avro version doesn't match the rest of the build - Its Flume version is not synced with the other Flume module - The other Flume module declares its dependency on Flume Sink slightly incorrectly, hard-coding the Scala 2.10 version - It depends on Scala Lang directly, which it shouldn't Author: Sean Owen <[email protected]> Closes apache#1726 from srowen/SPARK-2798 and squashes the following commits: a46e2c6 [Sean Owen] scalatest to test scope, harmonize Avro and Flume versions, remove direct Scala dependency, fix '2.10' in Flume dependency
…1727) This PR adds BosonSort handling in JoinSuite so that the tests will pass with Boson. This is the same change as apache#1725 and apache#1726
(EDIT) Since the scalatest issue was since resolved, this is now about a few small problems in the Flume Sink
pom.xmlscalatestis not declared as a test-scope dependency