Skip to content
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

Fix build warnings #28

Merged

Conversation

carlyeks
Copy link
Member

There were a few build errors that have been collecting. I've fixed them, and also made warnings fatal so that they are addressed earlier.

All of the changes were because of deprecation in an imported library; most were related to Hadoop's Job constructor being deprecated. The biggest change was in Genotype.getAttributeAsString. I don't know why it was deprecated, but I've included a scala version of it.

@AmplabJenkins
Copy link

Merged build triggered.

@AmplabJenkins
Copy link

Merged build started.

@AmplabJenkins
Copy link

Merged build finished.

@AmplabJenkins
Copy link

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/ADAM/102/

@@ -89,6 +89,7 @@
<arg>-unchecked</arg>
<arg>-optimise</arg>
<arg>-deprecation</arg>
<arg>-Xfatal-warnings</arg>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice flag addition. It will definitely force us to address warnings instead of ignore them. :)

Please indent the arg at the same level as the others.

@massie
Copy link
Member

massie commented Dec 12, 2013

Aside from the indentation nit, this looks ready to merge to me.

- Make future build warnings fatal, so that we can address them earlier
@AmplabJenkins
Copy link

Merged build triggered.

@AmplabJenkins
Copy link

Merged build started.

@AmplabJenkins
Copy link

Merged build finished.

@AmplabJenkins
Copy link

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/ADAM/105/

massie added a commit that referenced this pull request Dec 13, 2013
@massie massie merged commit d691289 into bigdatagenomics:master Dec 13, 2013
@massie
Copy link
Member

massie commented Dec 13, 2013

Thanks, Carl!

@tdanford tdanford deleted the carlyeks/buildWarnings branch December 13, 2013 11:42
@fnothaft
Copy link
Member

FYI - this breaks the build for those using Hadoop 1.0.4 (the default hadoop version for the Spark EC2 scripts).

[INFO] Compiling 52 Scala sources to /Users/fnothaft/adam-new/adam/adam-commands/target/scala-2.9.3/classes...
[ERROR] /Users/fnothaft/adam-new/adam/adam-commands/src/main/scala/edu/berkeley/cs/amplab/adam/commands/AdamCommand.scala:47: value getInstance is not a member of object org.apache.hadoop.mapreduce.Job
[ERROR] val job = Job.getInstance()
[ERROR] ^
[ERROR] /Users/fnothaft/adam-new/adam/adam-commands/src/main/scala/edu/berkeley/cs/amplab/adam/rdd/AdamContext.scala:106: value getInstance is not a member of object org.apache.hadoop.mapreduce.Job
[ERROR] val job = Job.getInstance(sc.hadoopConfiguration)
[ERROR] ^
[ERROR] /Users/fnothaft/adam-new/adam/adam-commands/src/main/scala/edu/berkeley/cs/amplab/adam/rdd/AdamContext.scala:116: value getInstance is not a member of object org.apache.hadoop.mapreduce.Job
[ERROR] val job = Job.getInstance(sc.hadoopConfiguration)
[ERROR] ^
[ERROR] /Users/fnothaft/adam-new/adam/adam-commands/src/main/scala/edu/berkeley/cs/amplab/adam/rdd/AdamContext.scala:210: value getInstance is not a member of object org.apache.hadoop.mapreduce.Job
[ERROR] val job = Job.getInstance(sc.hadoopConfiguration)
[ERROR] ^
[ERROR] /Users/fnothaft/adam-new/adam/adam-commands/src/main/scala/edu/berkeley/cs/amplab/adam/rdd/AdamRDDFunctions.scala:41: value getInstance is not a member of object org.apache.hadoop.mapreduce.Job
[ERROR] val job = Job.getInstance(rdd.context.hadoopConfiguration)
[ERROR] ^
[ERROR] /Users/fnothaft/adam-new/adam/adam-commands/src/main/scala/edu/berkeley/cs/amplab/adam/util/ParquetFileTraversable.scala:34: value isDirectory is not a member of org.apache.hadoop.fs.FileStatus
[ERROR] if (status.isDirectory) {
[ERROR] ^
[ERROR] /Users/fnothaft/adam-new/adam/adam-commands/src/main/scala/edu/berkeley/cs/amplab/adam/util/ParquetFileTraversable.scala:42: value isFile is not a member of org.apache.hadoop.fs.FileStatus
[ERROR] } else if (status.isFile) {
[ERROR] ^
[ERROR] /Users/fnothaft/adam-new/adam/adam-commands/src/main/scala/edu/berkeley/cs/amplab/adam/util/PileupTraversable.scala:86: value getInstance is not a member of object org.apache.hadoop.mapreduce.Job
[ERROR] val job = Job.getInstance()
[ERROR] ^
[ERROR] 8 errors found

@fnothaft
Copy link
Member

This also does not run out of the box with the spark-ec2 scripts with hadoop-major-version=2. Digging further...

@tdanford
Copy link
Contributor

So, two things -- is there a more automated way for us to test in this kind of environment?

Carl and I are in a meeting at the moment, but it sounds like he'll dig into this when we're done here. Frank, what's your preference, should we revert? Or fix it in some other way?

@fnothaft
Copy link
Member

Long term, I think that we need to figure out what versions of all tools we need to support. From there, we'll probably set up matrix builds in Jenkins to test this out.

I wouldn't fret from your side right now — I'm going through hadoop versions trying to sort out which version works. There's nothing really to do until then. Realistically, I think this problem will be resolved through documentation and testing, not a code patch.

TL;DR: Jenkins and documentation.

@massie
Copy link
Member

massie commented Dec 16, 2013

We can protect against this by updating our Jenkins job. I'll do that soon (possibly today).

@fnothaft
Copy link
Member

It seems that Spark requires hadoop-client:2.0.0-cdh4.2.0. The Maven jar for this does not still appear to be available: http://mvnrepository.com/artifact/org.apache.hadoop/hadoop-client

Thoughts? I assume this is a rough patch that is going to be fixed by spark-0.8.1?

@fnothaft
Copy link
Member

I've reverted this change in master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants