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

fixes related to GATK bump to 3.8 #867

Merged
merged 5 commits into from
Jul 24, 2018
Merged

fixes related to GATK bump to 3.8 #867

merged 5 commits into from
Jul 24, 2018

Conversation

notestaff
Copy link
Contributor

Removed GATK command-line args deprecated since 3.7 . Changed default path to GATK in pipes/config.yaml to point to a 3.8 version.

@@ -87,7 +87,7 @@ env_vars:
# The directory path to the location of the GATK jar file.
# It must be explicitly given since GATK has to be licensed and downloaded
# manually out of band.
GATK_PATH: "/humgen/gsa-hpprojects/GATK/bin/GenomeAnalysisTK-3.6-0-g89b7209"
GATK_PATH: "/idi/sabeti-scratch/shared-resources/software/gatk/GenomeAnalysisTK-3.8-0-ge9d806836"
Copy link
Member

Choose a reason for hiding this comment

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

Somewhat annoying there isn't an extracted directory for v3.8 within /humgen/gsa-hpprojects/GATK/bin. Should we ping someone upstairs?

Copy link
Member

@tomkinsc tomkinsc left a comment

Choose a reason for hiding this comment

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

@tomkinsc
Copy link
Member

Thanks for catching these!

@notestaff
Copy link
Contributor Author

@tomkinsc I wonder why the standard travis-ci testing didn't catch these?

@tomkinsc
Copy link
Member

The easy-deploy script is no longer tested; I think it was taking too long. Otherwise, we set the GATK_PATH on Travis.

@dpark01
Copy link
Member

dpark01 commented Jul 24, 2018

But the call to unified genotyper... that should've failed. Oh no, I know why... @tomkinsc when you updated GATK versions, did you also update the encrypted Travis tarball? Or is Travis still testing 3.6?

@dpark01
Copy link
Member

dpark01 commented Jul 24, 2018

Actually instead of pulling the GATK jar from an encrypted Travis secret, I wonder if we can just pull it now from the no-longer-auth-protected GATK public URL... at least for Travis...

@tomkinsc
Copy link
Member

Shoot, I haven't updated it yet.

@dpark01
Copy link
Member

dpark01 commented Jul 24, 2018

Yeah so... since the GATK team no longer protects their 3.x download URLs (and in fact, it looks like the gatk-protected github repo is now public?), we could replace this section of our install script with just wget https://software.broadinstitute.org/gatk/download/auth?package=GATK-archive&version=3.8-1-0-gf15c1c3ef (which, despite the "auth" in the URI, doesn't actually auth anymore).

@notestaff
Copy link
Contributor Author

@dpark01
Copy link
Member

dpark01 commented Jul 24, 2018

Yeah we don't use that repo anymore... not since last fall.

@tomkinsc
Copy link
Member

@notestaff: Could you incorporate the Travis GATK wget call into this PR?

@notestaff
Copy link
Contributor Author

Maybe, delete the viral-ngs-deploy repo, to avoid confusion? Or check in a final commit pointing to the new location. @tomkinsc @dpark01

@dpark01
Copy link
Member

dpark01 commented Jul 24, 2018

So actually... it looks like our travis setup no longer does anything with novoalign (we just test on the free/public version) so if we can get rid of the encrypted secret (like make the changes I suggested above and then also delete the three instances of BUNDLE_SECRET from .travis.yml) then people can actually run our python test suites on forks of our repo. Which would be nice.

@notestaff notestaff merged commit e7011c5 into master Jul 24, 2018
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.

3 participants