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.

Sign up for GitHub

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

Jump to bottom

VS-637 Address a couple of issues in SampleLoadStatus handling in GVSImportGenomes. #8052

Merged
merged 21 commits into from
Oct 19, 2022
Merged

VS-637 Address a couple of issues in SampleLoadStatus handling in GVSImportGenomes. #8052

merged 21 commits into from
Oct 19, 2022

Conversation

gbggrant
Copy link
Collaborator

@gbggrant gbggrant commented Oct 12, 2022 •
edited
Loading

This PR addresses some issues with tracking of sample loading in GVSImportGenomes.

  1. Due to a bug (unresolved as yet) where the status 'STARTED' can be written to sample_load_status multiple times, the method LoadStatus.getSampleLoadState would NEVER return COMPLETE. This PR fixes the logic to allow for the (unexpected) case of multiple 'STARTED' statuses.
  2. I have modified the SetIsLoadedColumn task in GVSImportGenomes to explicitly look for sample_load_status records with both STARTED and FINISHED to use to update sample_info.is_loaded to true.

@gbggrant
Change the Logic of LoadStatus.getSampleLoadState to allow for mutlip…
538b8dd 
…le STARTED and FINISHED records.

Also, have it run the query NOT in batch mode.
@codecov
Copy link

codecov bot commented Oct 12, 2022 •
edited
Loading

Codecov Report

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

Additional details and impacted files 
@@               Coverage Diff                @@
##             ah_var_store     #8052   +/-   ##
================================================
  Coverage                ?   86.242%           
  Complexity              ?     35201           
================================================
  Files                   ?      2173           
  Lines                   ?    165041           
  Branches                ?     17794           
================================================
  Hits                    ?    142334           
  Misses                  ?     16379           
  Partials                ?      6328

@gbggrant gbggrant marked this pull request as ready for review October 13, 2022 13:45
@gbggrant
Copy link
Collaborator Author

gbggrant commented Oct 13, 2022 •
edited
Loading

Successful run at:
https://job-manager.dsde-prod.broadinstitute.org/jobs/d6c68894-312a-4db8-b8d4-3c9d4f19c23d

@gbggrant gbggrant changed the title Change the Logic of LoadStatus.getSampleLoadState to allow for mutlip… VS-637 Address a couple of issues in SampleLoadStatus handling in GVSImportGenomes. Oct 13, 2022
@gatk-bot
Copy link

gatk-bot commented Oct 13, 2022 •
edited
Loading

Github actions tests reported job failures from actions build 3244925651
Failures in the following jobs:

Test Type JDK Job ID Logs
cloud 8 3244925651.10 logs
cloud 11 3244925651.11 logs

@gatk-bot
Copy link

gatk-bot commented Oct 13, 2022 •
edited
Loading

Github actions tests reported job failures from actions build 3245059454
Failures in the following jobs:

Test Type JDK Job ID Logs
cloud 8 3245059454.10 logs
cloud 11 3245059454.11 logs

@gatk-bot
Copy link

gatk-bot commented Oct 13, 2022 •
edited
Loading

Github actions tests reported job failures from actions build 3245400724
Failures in the following jobs:

Test Type JDK Job ID Logs
cloud 8 3245400724.10 logs
cloud 11 3245400724.11 logs

@gatk-bot
Copy link

gatk-bot commented Oct 13, 2022 •
edited
Loading

Github actions tests reported job failures from actions build 3245510955
Failures in the following jobs:

Test Type JDK Job ID Logs
cloud 8 3245510955.10 logs
cloud 11 3245510955.11 logs

@gbggrant
Copy link
Collaborator Author

retest this please

gbggrant
gbggrant commented Oct 18, 2022
View reviewed changes
scripts/variantstore/wdl/GvsImportGenomes.wdl Outdated
@@ -22,7 +22,7 @@ workflow GvsImportGenomes {
Int? load_data_batch_size
Int? load_data_preemptible_override
Int? load_data_maxretries_override
File? load_data_gatk_override = "gs://gvs_quickstart_storage/jars/gatk-package-4.2.0.0-613-g1219576-SNAPSHOT-local.jar"
File? load_data_gatk_override = "gs://gvs_quickstart_storage/jars/VS-637/gatk-package-4.2.0.0-614-g8f8f1691-SNAPSHOT-local.jar"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Note - will update this (put the jar in the 'standard' location) at merge time.

rsasch
rsasch approved these changes Oct 18, 2022
View reviewed changes
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

@gbggrant gbggrant merged commit f2dcc68 into ah_var_store Oct 19, 2022
@gbggrant gbggrant deleted the gg_VS-637_RespectSampleLoadStatusFinishedFlag branch October 19, 2022 00:29
This was referenced Mar 17, 2023
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rsasch rsasch rsasch approved these changes

@mcovarr mcovarr Awaiting requested review from mcovarr

@RoriCremer RoriCremer Awaiting requested review from RoriCremer

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@gbggrant @gatk-bot @rsasch