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

[ADAM-1057] Remove workaround for gzip/BGZF compressed VCF headers #1060

Closed
wants to merge 1 commit into from

Conversation

heuermh
Copy link
Member

@heuermh heuermh commented Jun 28, 2016

Fixes #1057

Requires HadoopGenomics/Hadoop-BAM#106 to be merged and deployed as version 7.5.1-SNAPSHOT. Will need to be updated to 7.6.0 when that release is made.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/ADAM-prb/1289/

Build result: FAILURE

GitHub pull request #1060 of commit b6e8d3c automatically merged.Notifying endpoint 'HTTP:https://webhooks.gitter.im/e/ac8bb6e9f53357bc8aa8'[EnvInject] - Loading node environment variables.Building remotely on amp-jenkins-worker-05 (centos spark-test) in workspace /home/jenkins/workspace/ADAM-prb > /home/jenkins/git2/bin/git rev-parse --is-inside-work-tree # timeout=10Fetching changes from the remote Git repository > /home/jenkins/git2/bin/git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > /home/jenkins/git2/bin/git --version # timeout=10 > /home/jenkins/git2/bin/git -c core.askpass=true fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/pull/:refs/remotes/origin/pr/ # timeout=15 > /home/jenkins/git2/bin/git rev-parse origin/pr/1060/merge^{commit} # timeout=10 > /home/jenkins/git2/bin/git branch -a --contains 1b4111f # timeout=10 > /home/jenkins/git2/bin/git rev-parse remotes/origin/pr/1060/merge^{commit} # timeout=10Checking out Revision 1b4111f (origin/pr/1060/merge) > /home/jenkins/git2/bin/git config core.sparsecheckout # timeout=10 > /home/jenkins/git2/bin/git checkout -f 1b4111fec822b4840b82843a90e6915b0023b8c7First time build. Skipping changelog.Triggering ADAM-prb ? 2.6.0,2.10,1.5.2,centosTriggering ADAM-prb ? 2.6.0,2.11,1.5.2,centosTouchstone configurations resulted in FAILURE, so aborting...Notifying endpoint 'HTTP:https://webhooks.gitter.im/e/ac8bb6e9f53357bc8aa8'
Test FAILed.

@heuermh
Copy link
Member Author

heuermh commented Jun 30, 2016

Jenkins, test this please.

Issue HadoopGenomics/Hadoop-BAM#106 has been closed and a new snapshot should have been deployed.

@heuermh
Copy link
Member Author

heuermh commented Jul 1, 2016

Jenkins, retest this please.

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/ADAM-prb/1293/
Test PASSed.

@@ -25,12 +25,12 @@
<parquet.version>1.8.1</parquet.version>
<!-- Edit the following line to configure the Hadoop (HDFS) version. -->
<hadoop.version>2.6.0</hadoop.version>
<hadoop-bam.version>7.5.0</hadoop-bam.version>
<hadoop-bam.version>7.5.1-SNAPSHOT</hadoop-bam.version>
Copy link
Member

Choose a reason for hiding this comment

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

This will go to 7.5.1 when that pushes to Maven Central, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

7.6.0 was just released and has made it to Maven Central, so I'll push & rebase with the new version shortly

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/ADAM-prb/1294/
Test PASSed.

@@ -59,7 +59,7 @@ import org.bdgenomics.utils.io.LocalFileByteAccess
import org.bdgenomics.utils.misc.HadoopUtil
import org.bdgenomics.utils.misc.Logging
import org.seqdoop.hadoop_bam._
import org.seqdoop.hadoop_bam.util.{ BGZFCodec, SAMHeaderReader, VCFHeaderReader, WrapSeekable }
import org.seqdoop.hadoop_bam.util.{ BGZFCodec, BGZFEnhancedGzipCodec, SAMHeaderReader, VCFHeaderReader, WrapSeekable }
Copy link
Member

Choose a reason for hiding this comment

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

Small nit for readability: can you change this to either org.seqdoop.hadoop_bam.util._ or split it up across multiple lines? I think we typically go to importing the whole package at 4 imports.

@fnothaft
Copy link
Member

fnothaft commented Jul 4, 2016

LGTM, modulo one import nit. Let me know when you've updated it, and I can merge. I'm looking forward to having this in, @heuermh!

@fnothaft
Copy link
Member

fnothaft commented Jul 5, 2016

Hmm, I'm having bad times running this on the cluster on the 1KG .vcf.gz files. Unfortunately, I'm getting an uninformative error—the exception isn't serializable, which throws yet another exception—let me try to debug that and get a useful error.

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/ADAM-prb/1299/
Test PASSed.

@heuermh heuermh modified the milestone: 0.20.0 Jul 18, 2016
@heuermh heuermh changed the title Remove workaround for gzip/BGZF compressed VCF headers [ADAM-1057] Remove workaround for gzip/BGZF compressed VCF headers Jul 18, 2016
@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/ADAM-prb/1349/
Test PASSed.

@heuermh
Copy link
Member Author

heuermh commented Jul 19, 2016

Rebased!

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/ADAM-prb/1352/
Test PASSed.

@jpdna
Copy link
Member

jpdna commented Jul 26, 2016

+1 This works fine for me on a compressed vcf
I'll merge tomorrow unless someone suggests otherwise

@jpdna
Copy link
Member

jpdna commented Jul 26, 2016

Merged 308cc58
Thanks for the PR @heuermh!

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.

4 participants