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

Removed usage of service account from WDLs #7985

Merged
merged 8 commits into from
Aug 16, 2022
Merged

Conversation

gbggrant
Copy link
Collaborator

Removed usage of service account from:
GvsAssignIds.wdl
GvsImportGenomes.wdl
GvsCreateAltAllele.wdl
GvsCreateFilterSet.wdl
GvsWithdrawSamples.wdl
GvsCreateAltAllele.wdl
GvsPrepareRangesCallset.wdl
GvsExtractCallset.wdl
GvsExtractCohortFromSampleNames.wdl
GvsUnified.wdl
GvsJointVariantCalling.wdl
GvsAoUReblockGvcf.wdl
GvsBenchmarkExtractTask.wdl
GvsRescatterCallsetInterval.wdl

…enomes.wdl

Removed usage of service account from GvsCreateAltAllele.wdl, GvsCreateFilterSet.wdl, and GvsWithdrawSamples.wdl
Removing missed usages of service account in GvsCreateAltAllele.wdl and related python script
Removed usage of service account from GvsPrepareRangesCallset.wdl, GvsExtractCallset.wdl, and GvsExtractCohortFromSampleNames.wdl - also removed from related python script, rebuilt docker and updated all usages of ah_var_store_ docker to the test build.
Removed service account usage from GvsUnified.wdl and GvsJointVariantCalling.wdl
Removed usage of service_account from GvsAoUReblockGvcf.wdl, GvsBenchmarkExtractTask.wdl, and GvsRescatterCallsetInterval.wdl
@gbggrant
Copy link
Collaborator Author

gbggrant commented Aug 10, 2022

In order to test this, I created a small (10 sample) test set using all of us data. That is here
Here are successful runs of the following wdls:
GvsAssignIds
GvsImportGenomes
GvsCreateAltAllele
GvsCreateFilterSet
GvsPrepareRangesCallSet
GvsExtractCallSet
GvsCreateVAT
GvsValidateVAT
GvsUnified
GvsJointVariantCalling
GvsRescatterCallsetInterval - Failed but got beyond the point where it would need the service account.
GvsBenchmarkExtractTest - Failed, but got beyond the point where it would need the service account.
GvsCallsetCost (unaffected, but tested anyway)
GvsAoUReblockGvcf - Tested by Morgan
GVSCacluatedPrecisionAndSensitivity (unaffected)
GvsJointVariantCallingCallsetCost (unaffected)
GvsQuickstartIntegration (unaffected)

@codecov
Copy link

codecov bot commented Aug 10, 2022

Codecov Report

❗ No coverage uploaded for pull request base (ah_var_store@42a9382). Click here to learn what that means.
The diff coverage is n/a.

@@               Coverage Diff                @@
##             ah_var_store     #7985   +/-   ##
================================================
  Coverage                ?   86.243%           
  Complexity              ?     35203           
================================================
  Files                   ?      2173           
  Lines                   ?    165016           
  Branches                ?     17792           
================================================
  Hits                    ?    142315           
  Misses                  ?     16376           
  Partials                ?      6325           

Copy link
Collaborator

@mcovarr mcovarr left a comment

Choose a reason for hiding this comment

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

yesss 👍 👍 👍 (once all the WDLs have tested out)

Copy link

@rsasch rsasch left a comment

Choose a reason for hiding this comment

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

LGTM! I would definitely test GvsAoUReblockGvcf.wdl to make sure that we don't need the SA to access where the un-blocked gVCFs are. And we should probably give AoU (Tarek/Shimon) a heads up that the ExtractCohortFromSampleNames WDL won't have the SA anymore (I don't know how it was run in the past).

@mmorgantaylor
Copy link
Member

Confirmed that GvsAoUReblockGvcf.wdl works 👍

@gbggrant gbggrant merged commit 0130bb8 into ah_var_store Aug 16, 2022
@gbggrant gbggrant deleted the gg_VS-444_NoSa branch August 16, 2022 15:08
This was referenced Mar 17, 2023
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