Job Add labels to BQ operations from GATK (Issues-199) #7115

Marianie-Simeon · 2021-03-01T15:57:28Z

Addresses

https://github.com/broadinstitute/dsp-spec-ops/issues/199

Commit Summary

-Add labels to Sample list, Extract Cohort, and Extract features

Edit Bigquery Utils Testing
Add labels as an input to a Bigquery functions
Created UID class for a unique identifier

Output

For Sample list, Extract Cohort and Extract features

"query", "Run_SampleTable_<UID>"
"query", "extract_cohort_<UID>"
"query", "extract_features_<UID>"

Testing:

"test_query", "get_all_records_<UID>"
"test_query", "test_where_clause_<UID>"
"test_query", "test_batch_mode_<UID>"
"test_query", "test_specified_execute_query_<UID>"
"test_query", "test_storage_api_<UID>"
"test_query", "test_storage_api_with_empty_dataset_<UID>"

kcibul

Looking good -- comments left within. I think there's still some work to be done in the calling classes, let's work with the following labels:

label -> value
gatk_tool -> FQN of the GATK tool, can be obtained in the tool constructor
gatk_execution_id -> UUID, can be generated in the tool constructor (e.g. ExtractCohortEngine)

(multiple) -> passed in to tool as a (list?) of key-value pairs. We just propagate to the tool. We'll set these in our WDLs to be consistent (ie joint-calling-wdl-id)

I don't think each query itself needs it's own UUID (ie if I call SampleList 5 times, should there be 5 unique ids for that?). I could be convinced otherwise though.

kcibul · 2021-03-02T00:59:17Z

src/main/java/org/broadinstitute/hellbender/utils/bigquery/BigQueryUtils.java

-    public static TableResult executeQuery(final String queryString, final boolean runQueryInBatchMode) {
-        return executeQuery(getBigQueryEndPoint(), queryString, runQueryInBatchMode);
+
+    // TODO: add Collections.EMPTY_MAP


Is this still a todo? or can this be deleted?

It can be deleted

src/main/java/org/broadinstitute/hellbender/utils/bigquery/BigQueryUtils.java

kcibul · 2021-03-02T01:03:49Z

src/main/java/org/broadinstitute/hellbender/utils/bigquery/BigQueryUtils.java

@@ -384,7 +394,9 @@ public static StorageAPIAvroReader executeQueryWithStorageAPI(final String query
                ") AS\n" +
                queryString;

-        executeQuery(queryStringIntoTempTable, runQueryInBatchMode);
+        // TODO: add label


lol, I going to use this from now on. I laughed way too hard at this

src/main/java/org/broadinstitute/hellbender/utils/bigquery/UID.java

src/test/java/org/broadinstitute/hellbender/utils/bigquery/BigQueryUtilsUnitTest.java

gatk-bot · 2021-03-09T20:24:13Z

Travis reported job failures from build 33100
Failures in the following jobs:

Test Type	JDK	Job ID	Logs
cloud	openjdk8	33100.1	logs
cloud	openjdk11	33100.14	logs

Add labels to Sample list, Extract Cohort, and Extract features

Editing cdode according to the comments

gatk-bot · 2021-03-09T21:09:57Z

Travis reported job failures from build 33109
Failures in the following jobs:

Test Type	JDK	Job ID	Logs
cloud	openjdk8	33109.1	logs
cloud	openjdk11	33109.14	logs

Add labels to Sample list, Extract Cohort, and Extract features

Editing cdode according to the comments

kcibul · 2021-03-10T14:27:07Z

Your tests are failing because the labels generated are invalid. Here are the rules for labels: https://cloud.google.com/bigquery/docs/labels-intro

kcibul

It doesn't look like you saw my main comment from the last review (ie not attached to a line of code). Specifically around "label -> value". If this is feeling too big, maybe let's separate this PR into to pieces.

Changes the BigQueryUtils to support labels, and callers to pass null in for the labels
Changes to callers to pass in the right sets of labels

wdyt?

src/main/java/org/broadinstitute/hellbender/utils/bigquery/BigQueryUtils.java

src/test/java/org/broadinstitute/hellbender/utils/bigquery/BigQueryUtilsUnitTest.java

src/main/java/org/broadinstitute/hellbender/tools/variantdb/nextgen/ExtractCohortEngine.java

gatk-bot · 2021-03-10T14:47:09Z

Travis reported job failures from build 33118
Failures in the following jobs:

Test Type	JDK	Job ID	Logs
cloud	openjdk8	33118.1	logs
cloud	openjdk11	33118.14	logs

Change labels to null to make the PR focus on BQ labels

…te/gatk into ms_add_labels_to_BQ

Clean up, base of PR comments

gatk-bot · 2021-03-11T21:26:00Z

Travis reported job failures from build 33170
Failures in the following jobs:

Test Type	JDK	Job ID	Logs
cloud	openjdk11	33170.14	logs
cloud	openjdk8	33170.1	logs

gatk-bot · 2021-03-11T21:27:26Z

Travis reported job failures from build 33172
Failures in the following jobs:

Test Type	JDK	Job ID	Logs
cloud	openjdk8	33172.1	logs
cloud	openjdk11	33172.14	logs

gatk-bot · 2021-03-11T21:38:49Z

Travis reported job failures from build 33175
Failures in the following jobs:

Test Type	JDK	Job ID	Logs
cloud	openjdk11	33175.14	logs
cloud	openjdk8	33175.1	logs

kcibul · 2021-03-15T17:40:08Z

src/main/java/org/broadinstitute/hellbender/tools/variantdb/nextgen/ExtractCohortEngine.java

@@ -328,8 +328,8 @@ private double getQUALapproxFromSampleRecord(GenericRecord sampleRecord) {
        // Non-AS QualApprox (used for qualapprox filter) is simply the sum of the AS values (see GnarlyGenotyper)
        if (s.contains("|")) {

-            // take the sum of all non-* alleles
-            // basically if our alleles are '*,T' or 'G,*' we want to ignore the * part            
+            // take the average of all non-* alleles


why did this change? I don't think it's an average

I'm not sure, why that changed

kcibul · 2021-03-15T17:41:47Z

src/main/java/org/broadinstitute/hellbender/tools/variantdb/SampleList.java

-        final TableResult result = BigQueryUtils.executeQuery(BigQueryUtils.getBigQueryEndPoint(executionProjectId) , sampleListQueryString, false);
+
+        // Execute the query:
+        final TableResult result = BigQueryUtils.executeQuery(sampleListQueryString, null);


Change this back to the invocation that supplied the projectId, but also includes your null for the labels. e.g.

final TableResult result = BigQueryUtils.executeQuery(BigQueryUtils.getBigQueryEndPoint(executionProjectId) , sampleListQueryString, false, null);

gatk-bot · 2021-03-15T18:27:12Z

Travis reported job failures from build 33193
Failures in the following jobs:

Test Type	JDK	Job ID	Logs
cloud	openjdk8	33193.1	logs
cloud	openjdk11	33193.14	logs

gatk-bot · 2021-03-15T21:27:11Z

Travis reported job failures from build 33210
Failures in the following jobs:

Test Type	JDK	Job ID	Logs
cloud	openjdk11	33210.14	logs
cloud	openjdk8	33210.1	logs

gatk-bot · 2021-03-16T15:20:31Z

Travis reported job failures from build 33221
Failures in the following jobs:

Test Type	JDK	Job ID	Logs
cloud	openjdk11	33221.14	logs
cloud	openjdk8	33221.1	logs

gatk-bot · 2021-03-18T14:01:23Z

Travis reported job failures from build 33253
Failures in the following jobs:

Test Type	JDK	Job ID	Logs
cloud	openjdk8	33253.1	logs
cloud	openjdk8	33253.1	logs

kcibul

lgtm!

* Added labels to Big Query Job Add labels to Sample list, Extract Cohort, and Extract features * Update BigQueryUtilsUnitTest.java * Cleaning up code Editing cdode according to the comments * Added labels to Big Query Job Add labels to Sample list, Extract Cohort, and Extract features * Update BigQueryUtilsUnitTest.java * Cleaning up code Editing cdode according to the comments * Change piplines labels to null Change labels to null to make the PR focus on BQ labels * update labels * Clean code Clean up, base of PR comments * Update date labels in test class * Update labels with out ':' and edit comment and code for PR * Remove underscores from labels * Shorten label * test GRADLE * Format code Co-authored-by: Marianie-Simeon <[email protected]>

kcibul requested changes Mar 2, 2021

View reviewed changes

Marianie-Simeon mentioned this pull request Mar 4, 2021

Created AvroFileReader #7127

Closed

kcibul force-pushed the ah_var_store branch from 76da027 to f3134df Compare March 9, 2021 17:41

Marianie-Simeon force-pushed the ms_add_labels_to_BQ branch from 056d16c to 659abf4 Compare March 9, 2021 18:43

Marianie-Simeon closed this Mar 9, 2021

Marianie-Simeon reopened this Mar 9, 2021

Marianie-Simeon force-pushed the ms_add_labels_to_BQ branch from 659abf4 to ee01670 Compare March 9, 2021 19:53

Marianie-Simeon added 3 commits March 9, 2021 15:39

Added labels to Big Query Job

5af6876

Add labels to Sample list, Extract Cohort, and Extract features

Update BigQueryUtilsUnitTest.java

81fa02d

Cleaning up code

dc8057d

Editing cdode according to the comments

Marianie-Simeon force-pushed the ms_add_labels_to_BQ branch from ee01670 to dc8057d Compare March 9, 2021 20:39

Marianie-Simeon added 3 commits March 10, 2021 09:14

Added labels to Big Query Job

c8dac99

Add labels to Sample list, Extract Cohort, and Extract features

Update BigQueryUtilsUnitTest.java

a69f802

Cleaning up code

3d6911f

Editing cdode according to the comments

kcibul force-pushed the ms_add_labels_to_BQ branch from dc8057d to 3d6911f Compare March 10, 2021 14:16

kcibul requested changes Mar 10, 2021

View reviewed changes

Marianie-Simeon added 4 commits March 11, 2021 15:49

Change piplines labels to null

f032895

Change labels to null to make the PR focus on BQ labels

Merge branch 'ms_add_labels_to_BQ' of https://github.com/broadinstitu…

5d96c06

…te/gatk into ms_add_labels_to_BQ

update labels

fccedcc

Clean code

10d39e9

Clean up, base of PR comments

kcibul requested changes Mar 15, 2021

View reviewed changes

Update date labels in test class

9d6a339

Update labels with out ':' and edit comment and code for PR

77d6504

Marianie-Simeon added 2 commits March 16, 2021 10:46

Remove underscores from labels

b20c5aa

Shorten label

f7fa9a3

Marianie-Simeon added 2 commits March 16, 2021 12:34

test GRADLE

469d4ec

Format code

e1cc30c

Marianie-Simeon requested a review from kcibul March 18, 2021 17:19

kcibul approved these changes Mar 19, 2021

View reviewed changes

Marianie-Simeon merged commit c7dcac4 into ah_var_store Mar 19, 2021

Marianie-Simeon deleted the ms_add_labels_to_BQ branch March 19, 2021 18:23

This was referenced Mar 17, 2023

lb merge gvs branch #8248

Closed

testing something, please ignore #8251

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Job Add labels to BQ operations from GATK (Issues-199) #7115

Job Add labels to BQ operations from GATK (Issues-199) #7115

Marianie-Simeon commented Mar 1, 2021 •

edited

Loading

kcibul left a comment

kcibul Mar 2, 2021

Marianie-Simeon Mar 4, 2021

kcibul Mar 2, 2021

Marianie-Simeon Mar 4, 2021

gatk-bot commented Mar 9, 2021 •

edited

Loading

gatk-bot commented Mar 9, 2021 •

edited

Loading

kcibul commented Mar 10, 2021

kcibul left a comment

gatk-bot commented Mar 10, 2021 •

edited

Loading

gatk-bot commented Mar 11, 2021 •

edited

Loading

gatk-bot commented Mar 11, 2021 •

edited

Loading

gatk-bot commented Mar 11, 2021 •

edited

Loading

kcibul Mar 15, 2021

Marianie-Simeon Mar 15, 2021 •

edited

Loading

kcibul Mar 15, 2021

gatk-bot commented Mar 15, 2021 •

edited

Loading

gatk-bot commented Mar 15, 2021 •

edited

Loading

gatk-bot commented Mar 16, 2021 •

edited

Loading

gatk-bot commented Mar 18, 2021 •

edited

Loading

kcibul left a comment

Job Add labels to BQ operations from GATK (Issues-199) #7115

Job Add labels to BQ operations from GATK (Issues-199) #7115

Conversation

Marianie-Simeon commented Mar 1, 2021 • edited Loading

Addresses

Commit Summary

Output

kcibul left a comment

Choose a reason for hiding this comment

kcibul Mar 2, 2021

Choose a reason for hiding this comment

Marianie-Simeon Mar 4, 2021

Choose a reason for hiding this comment

kcibul Mar 2, 2021

Choose a reason for hiding this comment

Marianie-Simeon Mar 4, 2021

Choose a reason for hiding this comment

gatk-bot commented Mar 9, 2021 • edited Loading

gatk-bot commented Mar 9, 2021 • edited Loading

kcibul commented Mar 10, 2021

kcibul left a comment

Choose a reason for hiding this comment

gatk-bot commented Mar 10, 2021 • edited Loading

gatk-bot commented Mar 11, 2021 • edited Loading

gatk-bot commented Mar 11, 2021 • edited Loading

gatk-bot commented Mar 11, 2021 • edited Loading

kcibul Mar 15, 2021

Choose a reason for hiding this comment

Marianie-Simeon Mar 15, 2021 • edited Loading

Choose a reason for hiding this comment

kcibul Mar 15, 2021

Choose a reason for hiding this comment

gatk-bot commented Mar 15, 2021 • edited Loading

gatk-bot commented Mar 15, 2021 • edited Loading

gatk-bot commented Mar 16, 2021 • edited Loading

gatk-bot commented Mar 18, 2021 • edited Loading

kcibul left a comment

Choose a reason for hiding this comment

Marianie-Simeon commented Mar 1, 2021 •

edited

Loading

gatk-bot commented Mar 9, 2021 •

edited

Loading

gatk-bot commented Mar 9, 2021 •

edited

Loading

gatk-bot commented Mar 10, 2021 •

edited

Loading

gatk-bot commented Mar 11, 2021 •

edited

Loading

gatk-bot commented Mar 11, 2021 •

edited

Loading

gatk-bot commented Mar 11, 2021 •

edited

Loading

Marianie-Simeon Mar 15, 2021 •

edited

Loading

gatk-bot commented Mar 15, 2021 •

edited

Loading

gatk-bot commented Mar 15, 2021 •

edited

Loading

gatk-bot commented Mar 16, 2021 •

edited

Loading

gatk-bot commented Mar 18, 2021 •

edited

Loading