[GH-1221] import snapshots into a workspace #347

ehigham · 2021-03-19T16:17:22Z

RR: https://broadinstitute.atlassian.net/browse/GH-1221
In this PR, I'm adding test demonstration code for how to import data from a TDR snapshot into a Terra Workspace. This includes:

Querying all the rows in the specified table in the snapshot
Selecting and renaming the columns of the table to match those of the target entity
Importing the rows as new entities into the workspace.

The renaming works by viewing the rows of the table as a list of maps, mapping those names onto workspace names, then reconstructing the table.
Added dataset/snapshot querying utilities to the datarepo namespace.

Bug fixes:

"flattening" the big-query rows didn't flatten rows of arrays

…` implementation for REPEATED data types

and an `updated` datetime. Add REST wrappers for getting/deleting entities in a workspace Implement test for importing a snapshot into a workspace. Won't work until the dataset above is rebuild.

api/test/wfl/integration/datarepo_test.clj

rexwangcc

This looks great! Thanks for adding the import functionalities!! I left some questions, mainly for my own understanding

api/src/wfl/service/datarepo.clj

api/src/wfl/service/google/bigquery.clj

api/src/wfl/util.clj

rexwangcc · 2021-03-31T15:52:32Z

api/src/wfl/util.clj

+              (throw (TimeoutException. "Max number of attempts exceeded")))
+            (log/debugf "Sleeping - attempt #%s" attempt)
+            (.sleep TimeUnit/MILLISECONDS 100)
+            (recur (inc attempt)))))))


could you help explain the use case of this function? it looks like this is more like "check thrice in 0.3 seconds, throw or proceed with thunk" than a regular polling to me. (i guess it's the short sleep time and the small attempt number cap got me?)

Because importing the entity then getting the entity sometimes returns empty. This code tries at most 3 times to call the get. I've just pulled out the thunk.

Because importing the entity then getting the entity sometimes returns empty.

That is weird… So the point of this function is more like repeat more times than wait?

No, the goal is to poll the endpoint until the new entity is visible. It'll call at most 3 times.

api/test/wfl/integration/datarepo_test.clj

api/test/wfl/tools/snapshots.clj

api/test/wfl/unit/google/bigquery_test.clj

tbl3rd

Thank you for figuring this out.

api/src/wfl/service/datarepo.clj

tbl3rd · 2021-03-31T17:33:45Z

api/src/wfl/service/datarepo.clj

@@ -74,24 +77,23 @@
   "ingest"
   dataset-id
   {:format                "json"
-    :load_tag              "string"
+    :load_tag              (new-load-tag)
    :max_bad_records       0
    :path                  path
    :table                 table}))


Odd formatting, but old.

How could it be improved?

api/src/wfl/service/datarepo.clj

api/test/wfl/integration/datarepo_test.clj

api/src/wfl/util.clj

tbl3rd · 2021-03-31T19:16:50Z

api/test/wfl/integration/datarepo_test.clj

+                             (map :name)
+                             set)]
+              (doseq [[_ name] entities] (is (contains? names name))))
+            (finally (firecloud/delete-entities workspace entities))))))))


This could (keep :name) and drop contains?.

Wouldn't (keep :name) and (map :name) both deliver the same thing?

api/test/wfl/unit/google/bigquery_test.clj

okotsopoulos · 2021-04-01T20:02:23Z

api/src/wfl/service/datarepo.clj

+                       #{"running"})]
+     (while (running?) (.sleep TimeUnit/SECONDS seconds))
+     (result)))
+  ([job-id]


Clojure q - is a multi-arity function the best / only way to provide default argument(s)? What about the case where you might wish to allow multiple default arguments?

It's certainly one way, not sure if it's the best way. You can simulate named arguments by taking key-value pairs. Here's one way to implement that. There other ways and I'm not sure this is the best:

(let [defaults {:foo 'value1 :bar 'value2 :baz 'value3}] (defn my-named-args-function [& options] (let [inputs (apply assoc defaults options)] (use inputs)))) ;; usage (my-named-args-function :bar 12 :baz pi)

ehigham added 8 commits March 18, 2021 20:36

update sarscov2_illumina_full_inputs dataset to match flowcell values

b018685

remove trailing slash from tdr url

286264b

add/refactor querying for TDR dataset via BigQuery. Fix `flatten-rows…

358a0a6

…` implementation for REPEATED data types

start implementing importing into a workspace

68a3ae7

inline concat of firecloud route

7880563

fix error in docstring

7dd4d8d

Update sarscov2_illumina_full_inputs dataset to include flowcell_id

b60220b

and an `updated` datetime. Add REST wrappers for getting/deleting entities in a workspace Implement test for importing a snapshot into a workspace. Won't work until the dataset above is rebuild.

format

bcbb75c

ehigham marked this pull request as ready for review March 30, 2021 18:20

rexwangcc self-requested a review March 30, 2021 20:08

ehigham added 4 commits March 30, 2021 21:27

use timestamps instead of datetime for timezones

8fc86b1

update tests for new dataset + try to fix failures

c5fe494

fix unit test

a8f1cb2

use temporary workspaces. fix import test. fix bigquery table->csv.

8c46330

ehigham changed the title ~~[GH-1221] import snapshots to workspace~~ [GH-1221] import snapshots into a workspace Mar 31, 2021

fix tests again

2b8d226

ehigham force-pushed the ehigham/GH-1221-import-snapshots-to-workspace branch from e456f28 to 2b8d226 Compare March 31, 2021 01:56

use threading instead of >>>

211bccd

ehigham commented Mar 31, 2021

View reviewed changes

api/test/wfl/integration/datarepo_test.clj Outdated Show resolved Hide resolved

rexwangcc reviewed Mar 31, 2021

View reviewed changes

tbl3rd self-assigned this Mar 31, 2021

tbl3rd approved these changes Mar 31, 2021

View reviewed changes

ehigham added 3 commits April 1, 2021 16:53

address feedback after review

a357b47

pass in columns to table import code

0e1ee3e

fix some docstring

9485e93

rexwangcc approved these changes Apr 1, 2021

View reviewed changes

okotsopoulos reviewed Apr 1, 2021

View reviewed changes

reuse polling logic. remove duplicate fircloud function

09984ca

ehigham merged commit e3d500d into main Apr 2, 2021

ehigham deleted the ehigham/GH-1221-import-snapshots-to-workspace branch April 2, 2021 17:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[GH-1221] import snapshots into a workspace #347

[GH-1221] import snapshots into a workspace #347

ehigham commented Mar 19, 2021 •

edited

Loading

rexwangcc left a comment

rexwangcc Mar 31, 2021

ehigham Mar 31, 2021 •

edited

Loading

rexwangcc Apr 1, 2021

ehigham Apr 1, 2021

tbl3rd left a comment

tbl3rd Mar 31, 2021

ehigham Apr 1, 2021

tbl3rd Mar 31, 2021

ehigham Apr 1, 2021

okotsopoulos Apr 1, 2021

ehigham Apr 1, 2021 •

edited

Loading

[GH-1221] import snapshots into a workspace #347

[GH-1221] import snapshots into a workspace #347

Conversation

ehigham commented Mar 19, 2021 • edited Loading

rexwangcc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ehigham Mar 31, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tbl3rd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ehigham Apr 1, 2021 • edited Loading

Choose a reason for hiding this comment

ehigham commented Mar 19, 2021 •

edited

Loading

ehigham Mar 31, 2021 •

edited

Loading

ehigham Apr 1, 2021 •

edited

Loading