Move VEP to Requester Pays #8268
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Adding "WIP" tag since I had a test failure from |
|
Ah, the mix of reference genomes is coming from me running all the "cluster_tests" for GRCh38 cluster as well. One of the tests hardcodes 37 |
|
Ok, all tests passing now. Removing WIP |
tpoterba
previously requested changes
Mar 9, 2020
| conf.extend_flag('initialization-actions', [deploy_metadata[f'vep-{args.vep}.sh']]) | ||
| sys.stderr.write("WARNING: Running VEP outside of the US is VERY expensive. Please contact us for support.") |
There was a problem hiding this comment.
"Please contact the Hail team on https://discuss.hail.is for support."
There was a problem hiding this comment.
(and in the message above)
tpoterba
approved these changes
Mar 9, 2020
danking
pushed a commit
to danking/hail
that referenced
this pull request
Sep 6, 2023
This appers to have been lost in hail-is#8268. There is no explicit mention of it and it was inconsistently removed (e.g. loftee in GRCh38 still has the `&`). I assume it was a mistake.
danking
added a commit
that referenced
this pull request
Sep 6, 2023
This appers to have been lost in #8268. There is no explicit mention of it and it was inconsistently removed (e.g. loftee in GRCh38 still has the `&`). I assume it was a mistake.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR changes our vep init scripts to pull from the new
hail-us-vepgoogle bucket, which is requester pays.I realized while doing this that my previous PR (#8253) really had nothing to do with supporting this, since that really just sets the configuration for the Google Cloud / Hadoop connector, which is not how we get vep data. I tried to enforce the rules from those command line arguments anyway by rejecting
--vepflag if they don't specify they're ok with requester pays and manually checking thathail-us-vepwas in their approved bucket list. But if a user was to specify the init scripts usinggcloud dataprocand didn't go throughhailctl, there'd be no catch to check if they were ok with requester pays. Perhaps there is some way we could set environment variables on the dataproc machines based on the--requester-pays-allow....flags and use those in the init scripts.I also expanded
make test-dataprocto test with a GRCh38 cluster as well, as we use separate scripts to make them and I'm uncomfortable with only testing one.