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 Race Condition, Add Support for Extract by Array of Sample Names (ie from a Sample Set) #7917

Merged
merged 6 commits into from
Jul 6, 2022

Conversation

kcibul
Copy link
Contributor

@kcibul kcibul commented Jun 24, 2022

No description provided.

@codecov
Copy link

codecov bot commented Jun 24, 2022

Codecov Report

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

❗ Current head d353628 differs from pull request most recent head 46e7b4c. Consider uploading reports for the commit 46e7b4c to get more accurate results

@@               Coverage Diff                @@
##             ah_var_store     #7917   +/-   ##
================================================
  Coverage                ?   86.288%           
  Complexity              ?     35194           
================================================
  Files                   ?      2170           
  Lines                   ?    164888           
  Branches                ?     17785           
================================================
  Hits                    ?    142278           
  Misses                  ?     16288           
  Partials                ?      6322           

call write_array_task {
input:
input_array = select_first([cohort_sample_names_array]),
docker = "gcr.io/google.com/cloudsdktool/cloud-sdk:305.0.0"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Pinning cloud SDK versions can help avoid bugs but this particular one is ~2 years old

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep - I'm just taking the same version we use in other parts of the pipelines. We may be old but at least were consistent 😆

@@ -34,10 +35,22 @@ workflow GvsExtractCohortFromSampleNames {
File? gatk_override
}

# writing the array to a file has to be done in a task
# https://support.terra.bio/hc/en-us/community/posts/360071465631-write-lines-write-map-write-tsv-write-json-fail-when-run-in-a-workflow-rather-than-in-a-task
Copy link
Collaborator

Choose a reason for hiding this comment

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

uh okay, TIL 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah -- def a bummer, but seems to be the way it is

@@ -8,7 +8,8 @@ import "GvsExtractCallset.wdl" as GvsExtractCallset
workflow GvsExtractCohortFromSampleNames {

input {
File cohort_sample_names
File? cohort_sample_names
Array[String]? cohort_sample_names_array
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe a comment here that cohort_sample_names will be ignored if cohort_sample_names_array is defined?

Copy link

Choose a reason for hiding this comment

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

+1

@kcibul kcibul merged commit 874d615 into ah_var_store Jul 6, 2022
@kcibul kcibul deleted the kc_variant_search_extract_wdl branch July 6, 2022 15:10
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.

3 participants