You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@spark.apache.org by srowen <gi...@git.apache.org> on 2014/02/12 14:29:31 UTC
[GitHub] incubator-spark pull request: SPARK-1084. Fix most build warnings
GitHub user srowen opened a pull request:
https://github.com/apache/incubator-spark/pull/586
SPARK-1084. Fix most build warnings
https://spark-project.atlassian.net/browse/SPARK-1084
I hope another boring tidy-up JIRA might be welcome. I'd like to fix most of the warnings that appear during build, so that developers don't become accustomed to them. The accompanying pull request contains a number of commits to quash most warnings observed through the `mvn` and `sbt` builds, although not all of them.
### FIXED!
`[WARNING] Parameter tasks is deprecated, use target instead`
Just a matter of updating <tasks> -> <target> in inline Ant scripts.
`WARNING: -p has been deprecated and will be reused for a different (but still very cool) purpose in ScalaTest 2.0. Please change all uses of -p to -R.`
Goes away with updating scalatest plugin -> 1.0-RC2
```
[WARNING] Note: /Users/srowen/Documents/incubator-spark/core/src/test/scala/org/apache/spark/JavaAPISuite.java uses unchecked or unsafe operations.
[WARNING] Note: Recompile with -Xlint:unchecked for details.
```
Mostly `@SuppressWarnings("unchecked")` but needed a few more things to reveal the warning source: <fork>true</fork> (also needd for <maxmem>) and version 3.1 of the plugin. In a few cases some declaration changes were appropriate to avoid warnings.
```
/Users/srowen/Documents/incubator-spark/core/src/main/scala/org/apache/spark/util/IndestructibleActorSystem.scala:25: warning: Could not find any member to link for "akka.actor.ActorSystem".
/**
^
```
Getting several scaladoc errors like this and I'm not clear why it can't find the type -- outside its module? Remove the links as they're evidently not linking anyway?
```
/Users/srowen/Documents/incubator-spark/repl/src/main/scala/org/apache/spark/repl/SparkIMain.scala:86: warning: Variable eval undefined in comment for class SparkIMain in class SparkIMain
```
$ has to be escaped as \$ in scaladoc, apparently
```
[WARNING] 'dependencyManagement.dependencies.dependency.exclusions.exclusion.artifactId' for org.apache.hadoop:hadoop-yarn-client:jar with value '*' does not match a valid id pattern. @ org.apache.spark:spark-parent:1.0.0-incubating-SNAPSHOT, /Users/srowen/Documents/incubator-spark/pom.xml, line 494, column 25
```
This one might need review.
This is valid Maven syntax, but, Maven still warns on it. I wanted to see if we can do without it.
These are trying to exclude:
- `org.codehaus.jackson`
- `org.sonatype.sisu.inject`
- `org.xerial.snappy`
`org.sonatype.sisu.inject` doesn't actually seem to be a dependency anyway. `org.xerial.snappy` is used by dependencies but the version seems to match anyway (1.0.5).
`org.codehaus.jackson` was intended to exclude 1.8.8, since Spark streaming wants 1.9.11 directly. But the exclusion is in the wrong place if so, since Spark depends straight on Avro, which is what brings in 1.8.8, still. (hadoop-client 1.0.4 includes Jackson 1.0.1, so that needs an exclusion, but the other Hadoop modules don't.)
HBase depends on 1.8.8 but figured it was intentional to leave that as it would not collide with Spark streaming. (?)
(I understand this varies by Hadoop version but confirmed this is all the same for 1.0.4, 0.23.7, 2.2.0.)
### NOT FIXED
```
[warn] /Users/srowen/Documents/incubator-spark/streaming/src/test/scala/org/apache/spark/streaming/InputStreamsSuite.scala:305: method connect in class IOManager is deprecated: use the new implementation in package akka.io instead
[warn] override def preStart = IOManager(context.system).connect(new InetSocketAddress(port))
```
Not confident enough to fix this.
`[WARNING] there were 6 feature warning(s); re-run with -feature for details`
Don't know enough Scala to address these, yet.
`[WARNING] We have a duplicate org/yaml/snakeyaml/scanner/ScannerImpl$Chomping.class in /Users/srowen/.m2/repository/org/yaml/snakeyaml/1.6/snakeyaml-1.6.jar`
Probably addressable by being more careful about how binaries are packed though this appear to be ignorable; two identical copies of the class are colliding.
`[WARNING] Zinc server is not available at port 3030 - reverting to normal incremental compile`
and
`[WARNING] JAR will be empty - no content was marked for inclusion!`
Apparently harmless warnings, but I don't know how to disable them.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/apache/incubator-spark FixWarnings
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-spark/pull/586.patch
----
commit 7f6879c601c929c884b4e1fcaf155b1740dc8881
Author: Sean Owen <so...@cloudera.com>
Date: 2014-02-11T14:37:03Z
Replace deprecated Ant <tasks> with <target>
commit c4f37ebd4c28fd46f1d4fff2613cfea8ac9fb2aa
Author: Sean Owen <so...@cloudera.com>
Date: 2014-02-11T14:37:58Z
Fix unescaped $, bad tag in scaladoc
commit c5befe1c47923a43fc11d8c7fa10f223eec4257c
Author: Sean Owen <so...@cloudera.com>
Date: 2014-02-11T14:38:23Z
Remove dead scaladoc links
commit c88a983ea686de4381f1f367a9f49faedaf7fcc1
Author: Sean Owen <so...@cloudera.com>
Date: 2014-02-11T14:38:57Z
Suppress and/or fix unchecked javac warnings
commit 5d86e49b7c8d6473ecb90248c022d2b59791254b
Author: Sean Owen <so...@cloudera.com>
Date: 2014-02-11T14:39:48Z
Fix scaladoc invocation warning, and enable javac warnings properly, with plugin config updates
commit 4093ed0e947c83f258106a07e0377e70c03535d8
Author: Sean Owen <so...@cloudera.com>
Date: 2014-02-11T18:55:57Z
Fix (?) Jackson exclusion, and remove apparently unneeded exclusions, all of which were causing build warnings
----
[GitHub] incubator-spark pull request: SPARK-1084. Fix most build warnings
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/incubator-spark/pull/586#issuecomment-34868638
Merged build finished.
[GitHub] incubator-spark pull request: SPARK-1084. Fix most build warnings
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/incubator-spark/pull/586#issuecomment-34940308
Done, I believe. Have another glance at it.
[GitHub] incubator-spark pull request: SPARK-1084. Fix most build warnings
Posted by aarondav <gi...@git.apache.org>.
Github user aarondav commented on a diff in the pull request:
https://github.com/apache/incubator-spark/pull/586#discussion_r9973707
--- Diff: project/SparkBuild.scala ---
@@ -340,7 +336,8 @@ object SparkBuild extends Build {
def streamingSettings = sharedSettings ++ Seq(
name := "spark-streaming",
libraryDependencies ++= Seq(
- "commons-io" % "commons-io" % "2.4"
+ "commons-io" % "commons-io" % "2.4",
+ "org.codehaus.jackson" % "jackson-mapper-asl" % "1.9.11"
--- End diff --
Looking through the dependency graph of master, I do not see any dependencies that include 1.9.11. 1.8.8 seems to be the version used. Why do you say streaming needs 1.9.11?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. To do so, please top-post your response.
If your project does not have this feature enabled and wishes so, or if the
feature is enabled but not working, please contact infrastructure at
infrastructure@apache.org or file a JIRA ticket with INFRA.
---
[GitHub] incubator-spark pull request: SPARK-1084. Fix most build warnings
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/incubator-spark/pull/586#issuecomment-34868552
Merged build triggered.
[GitHub] incubator-spark pull request: SPARK-1084. Fix most build warnings
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/incubator-spark/pull/586#issuecomment-35843240
OK I'm going to come back with two PRs. One will have the squashed final output of this PR, and the other will have the parts related to dependencies (which are now quite trivial I think).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. To do so, please top-post your response.
If your project does not have this feature enabled and wishes so, or if the
feature is enabled but not working, please contact infrastructure at
infrastructure@apache.org or file a JIRA ticket with INFRA.
---
[GitHub] incubator-spark pull request: SPARK-1084. Fix most build warnings
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/incubator-spark/pull/586#issuecomment-34869797
Merged build triggered.
[GitHub] incubator-spark pull request: SPARK-1084. Fix most build warnings
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/incubator-spark/pull/586#issuecomment-35803385
Merged build started.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. To do so, please top-post your response.
If your project does not have this feature enabled and wishes so, or if the
feature is enabled but not working, please contact infrastructure at
infrastructure@apache.org or file a JIRA ticket with INFRA.
---
[GitHub] incubator-spark pull request: SPARK-1084. Fix most build warnings
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/incubator-spark/pull/586#issuecomment-35803384
Merged build triggered.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. To do so, please top-post your response.
If your project does not have this feature enabled and wishes so, or if the
feature is enabled but not working, please contact infrastructure at
infrastructure@apache.org or file a JIRA ticket with INFRA.
---
[GitHub] incubator-spark pull request: SPARK-1084. Fix most build warnings
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/incubator-spark/pull/586#discussion_r9975107
--- Diff: project/SparkBuild.scala ---
@@ -340,7 +336,8 @@ object SparkBuild extends Build {
def streamingSettings = sharedSettings ++ Seq(
name := "spark-streaming",
libraryDependencies ++= Seq(
- "commons-io" % "commons-io" % "2.4"
+ "commons-io" % "commons-io" % "2.4",
+ "org.codehaus.jackson" % "jackson-mapper-asl" % "1.9.11"
--- End diff --
Also, then I don't see a particular reason to bother excluding jackson (1.8.8) dependencies from Hadoop. It could be a problem to have no Jackson at all. I can undo that.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. To do so, please top-post your response.
If your project does not have this feature enabled and wishes so, or if the
feature is enabled but not working, please contact infrastructure at
infrastructure@apache.org or file a JIRA ticket with INFRA.
---
[GitHub] incubator-spark pull request: SPARK-1084. Fix most build warnings
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/incubator-spark/pull/586#issuecomment-34940301
Merged build triggered.
[GitHub] incubator-spark pull request: SPARK-1084. Fix most build warnings
Posted by aarondav <gi...@git.apache.org>.
Github user aarondav commented on the pull request:
https://github.com/apache/incubator-spark/pull/586#issuecomment-34918907
Thanks for this! Did a quick pass and left some comments.
[GitHub] incubator-spark pull request: SPARK-1084. Fix most build warnings
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/incubator-spark/pull/586#issuecomment-34869799
Merged build started.
[GitHub] incubator-spark pull request: SPARK-1084. Fix most build warnings
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/incubator-spark/pull/586#issuecomment-34868554
Merged build started.
[GitHub] incubator-spark pull request: SPARK-1084. Fix most build warnings
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/incubator-spark/pull/586#issuecomment-35827729
@aarondav Sure, it's already split into commits, and one of them has the dependency changes: https://github.com/srowen/incubator-spark/commit/6f2f67974bfedd40bafccd77abd0860dcbba4061 Move this to another PR?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. To do so, please top-post your response.
If your project does not have this feature enabled and wishes so, or if the
feature is enabled but not working, please contact infrastructure at
infrastructure@apache.org or file a JIRA ticket with INFRA.
---
[GitHub] incubator-spark pull request: SPARK-1084. Fix most build warnings
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/incubator-spark/pull/586#discussion_r9975099
--- Diff: project/SparkBuild.scala ---
@@ -340,7 +336,8 @@ object SparkBuild extends Build {
def streamingSettings = sharedSettings ++ Seq(
name := "spark-streaming",
libraryDependencies ++= Seq(
- "commons-io" % "commons-io" % "2.4"
+ "commons-io" % "commons-io" % "2.4",
+ "org.codehaus.jackson" % "jackson-mapper-asl" % "1.9.11"
--- End diff --
This was just making the sbt build consistent with Maven. But yeah on second glance it does look like Streaming doesn't even use Jackson! This can be removed in both places. Commons IO is used. I'll wait on your comment about splitting into a PR to move forward with fixes like this in this PR.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. To do so, please top-post your response.
If your project does not have this feature enabled and wishes so, or if the
feature is enabled but not working, please contact infrastructure at
infrastructure@apache.org or file a JIRA ticket with INFRA.
---
[GitHub] incubator-spark pull request: SPARK-1084. Fix most build warnings
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/incubator-spark/pull/586#issuecomment-34935722
I have a new commit ready to go that addresses the comments, but before I
pull the trigger, see replies inline with some questions about how you'd
like to proceed.
On Wed, Feb 12, 2014 at 9:18 PM, Aaron Davidson <no...@github.com>wrote:
> Thanks for this! Did a quick pass and left some comments.
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/apache/incubator-spark/pull/586#issuecomment-34918907>
> .
>
[GitHub] incubator-spark pull request: SPARK-1084. Fix most build warnings
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/incubator-spark/pull/586#issuecomment-35619835
Hello all - I think all outstanding questions and comments had been addressed in the last round of comments last week. Before it goes too stale, thought I'd ping the issue to see if it looks good to go to you guys. Totally up to you what you want to take or not take and I'm happy to rebase / tweak as you like.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. To do so, please top-post your response.
If your project does not have this feature enabled and wishes so, or if the
feature is enabled but not working, please contact infrastructure at
infrastructure@apache.org or file a JIRA ticket with INFRA.
---
[GitHub] incubator-spark pull request: SPARK-1084. Fix most build warnings
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/incubator-spark/pull/586#issuecomment-35804725
All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/12809/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. To do so, please top-post your response.
If your project does not have this feature enabled and wishes so, or if the
feature is enabled but not working, please contact infrastructure at
infrastructure@apache.org or file a JIRA ticket with INFRA.
---
[GitHub] incubator-spark pull request: SPARK-1084. Fix most build warnings
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/incubator-spark/pull/586#issuecomment-34872114
Merged build finished.
[GitHub] incubator-spark pull request: SPARK-1084. Fix most build warnings
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/incubator-spark/pull/586#issuecomment-34872115
All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/12692/
[GitHub] incubator-spark pull request: SPARK-1084. Fix most build warnings
Posted by srowen <gi...@git.apache.org>.
Github user srowen closed the pull request at:
https://github.com/apache/incubator-spark/pull/586
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. To do so, please top-post your response.
If your project does not have this feature enabled and wishes so, or if the
feature is enabled but not working, please contact infrastructure at
infrastructure@apache.org or file a JIRA ticket with INFRA.
---
[GitHub] incubator-spark pull request: SPARK-1084. Fix most build warnings
Posted by aarondav <gi...@git.apache.org>.
Github user aarondav commented on the pull request:
https://github.com/apache/incubator-spark/pull/586#issuecomment-35820063
Do you think it would be possible to separate this into two patches, one for the code warnings and one for the build issues? Code warnings are relatively easy to clean up without issue (and thus merge quickly), but we need to be very careful with dependencies; i.e., we need to go in and confirm that every change is not adding or removing actual dependencies.
The dependencies changes can also be incorporated into a more general "clean up the build file" type patch, since I noticed that there may be other issues, like spark streaming including commons-io when it is already included in sharedSettings.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. To do so, please top-post your response.
If your project does not have this feature enabled and wishes so, or if the
feature is enabled but not working, please contact infrastructure at
infrastructure@apache.org or file a JIRA ticket with INFRA.
---
[GitHub] incubator-spark pull request: SPARK-1084. Fix most build warnings
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/incubator-spark/pull/586#issuecomment-34868641
One or more automated tests failed
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/12691/
[GitHub] incubator-spark pull request: SPARK-1084. Fix most build warnings
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/incubator-spark/pull/586#issuecomment-34940302
Merged build started.
[GitHub] incubator-spark pull request: SPARK-1084. Fix most build warnings
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/incubator-spark/pull/586#issuecomment-35804723
Merged build finished.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. To do so, please top-post your response.
If your project does not have this feature enabled and wishes so, or if the
feature is enabled but not working, please contact infrastructure at
infrastructure@apache.org or file a JIRA ticket with INFRA.
---
[GitHub] incubator-spark pull request: SPARK-1084. Fix most build warnings
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/incubator-spark/pull/586#issuecomment-34941773
All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/12698/
[GitHub] incubator-spark pull request: SPARK-1084. Fix most build warnings
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/incubator-spark/pull/586#issuecomment-34941772
Merged build finished.
[GitHub] incubator-spark pull request: SPARK-1084. Fix most build warnings
Posted by aarondav <gi...@git.apache.org>.
Github user aarondav commented on the pull request:
https://github.com/apache/incubator-spark/pull/586#issuecomment-35838441
Ah, great, that'll make it simple. We can only merge at the granularity of PRs, so it'd be great if you could split the dependency stuff into its own.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. To do so, please top-post your response.
If your project does not have this feature enabled and wishes so, or if the
feature is enabled but not working, please contact infrastructure at
infrastructure@apache.org or file a JIRA ticket with INFRA.
---