From 28fd41480b870f258230f4866bdf40c2a50f998e Mon Sep 17 00:00:00 2001 From: Avi Kelman Date: Mon, 24 May 2021 19:34:26 -0600 Subject: [PATCH] :zap: Only patch descendant visibility if needed (also adds dry_run) Descendant patching functions now use new hide/unhide_entities to only patch visibility of descendants if they don't already have the desired setting. They will now also return the list of kfids that are actually being toggled. --- README.md | 22 +++--- kf_utils/dataservice/descendants.py | 69 ++++++++++++++----- .../test_dataservice_descendants.py | 23 +++++-- 3 files changed, 83 insertions(+), 31 deletions(-) diff --git a/README.md b/README.md index 00d8f88..27d693b 100644 --- a/README.md +++ b/README.md @@ -115,7 +115,7 @@ d2 = find_descendants_by_filter( # List genomic files with contributions from these biospecimens that also have # contributions from biospecimens that aren't these promiscuous_gs = find_gfs_with_extra_contributors( - db_url or api_url, ["BS_11111111", "BS_22222222", "BS_33333333"] + db_url or api_url, ["BS_11111111", "BS_22222222", "BS_33333333"] ) ``` @@ -124,7 +124,7 @@ promiscuous_gs = find_gfs_with_extra_contributors( # Genomic files receive the specified acl. # This and unhide_descendants_by_filter are not symmetrical. hide_descendants_by_filter( - api_url, "families", {"study_id": "SD_DYPMEHHF", "visible": True}, gf_acl=["SD_DYPMEHHF", "phs001436.c999"], db_url=db_url + api_url, "families", {"study_id": "SD_DYPMEHHF", "visible": True}, gf_acl=["SD_DYPMEHHF", "phs001436.c999"], db_url=db_url, dry_run=False ) ``` @@ -133,7 +133,9 @@ hide_descendants_by_filter( # genomic files with additional contributing specimens if those specimens will remain # hidden. # This and hide_descendants_by_filter are not symmetrical. -unhide_descendants_by_filter(api_url, "families", {"study_id": "SD_DYPMEHHF", "visible": False}, db_url=db_url) +unhide_descendants_by_filter( + api_url, "families", {"study_id": "SD_DYPMEHHF", "visible": False}, db_url=db_url, dry_run=False +) ``` `descendants.py` also provides wrapper functions hiding/unhiding descendants by KF ID(s): @@ -143,7 +145,7 @@ unhide_descendants_by_filter(api_url, "families", {"study_id": "SD_DYPMEHHF", "v # specified acl. # This and unhide_descendants_by_kfids are not symmetrical. hide_descendants_by_kfids( - api_url, "families", ["FM_12345678", "FM_87654321"], gf_acl=["SD_DYPMEHHF", "phs001436.c999"], db_url=db_url + api_url, "families", ["FM_12345678", "FM_87654321"], gf_acl=["SD_DYPMEHHF", "phs001436.c999"], db_url=db_url, dry_run=False ) ``` @@ -151,7 +153,9 @@ hide_descendants_by_kfids( # Unhide these families and all of their descendants except for genomic files with # additional contributing specimens if those specimens will remain hidden. # This and hide_descendants_by_kfids are not symmetrical. -unhide_descendants_by_kfids(api_url, "families", ["FM_12345678", "FM_87654321"], db_url=db_url) +unhide_descendants_by_kfids( + api_url, "families", ["FM_12345678", "FM_87654321"], db_url=db_url, dry_run=False +) ``` #### [dataservice/patch.py](kf_utils/dataservice/patch.py) - Rapid patch submission @@ -167,16 +171,16 @@ host = "http://localhost:5000" # Patch the given KFIDs with the given changes patches = { - "PT_12345678": {"visible": True}, - "BS_99999999": {"participant_id": "PT_12345678", "visible": False} + "PT_12345678": {"visible": True}, + "BS_99999999": {"participant_id": "PT_12345678", "visible": False} } send_patches(host, patches) ``` ```Python things = [ - {"kf_id": "PT_11223344", "visible": True}, - {"kf_id": "PT_22334455", "visible": False} + {"kf_id": "PT_11223344", "visible": True}, + {"kf_id": "PT_22334455", "visible": False} ] def my_patch_func(thing): diff --git a/kf_utils/dataservice/descendants.py b/kf_utils/dataservice/descendants.py index 4ce02b4..6edd6c1 100644 --- a/kf_utils/dataservice/descendants.py +++ b/kf_utils/dataservice/descendants.py @@ -7,8 +7,7 @@ import psycopg2 import psycopg2.extras - -from kf_utils.dataservice.patch import hide_kfids, unhide_kfids +from kf_utils.dataservice.patch import hide_entities, unhide_entities from kf_utils.dataservice.scrape import yield_entities @@ -431,7 +430,7 @@ def find_descendants_by_filter( def hide_descendants_by_filter( - api_url, endpoint, filter, gf_acl=None, db_url=None + api_url, endpoint, filter, gf_acl=None, db_url=None, dry_run=False ): """ Be aware that this and unhide_descendants_by_filter are not symmetrical. @@ -442,13 +441,22 @@ def hide_descendants_by_filter( change. """ desc = find_descendants_by_filter( - api_url, endpoint, filter, False, db_url=db_url + api_url, + endpoint, + filter, + ignore_gfs_with_hidden_external_contribs=False, + kfids_only=False, + db_url=db_url, ) - for v in desc.values(): - hide_kfids(api_url, v, gf_acl) + changed = [] + for es in desc.values(): + changed.extend(hide_entities(api_url, es.values(), gf_acl, dry_run)) + return changed -def unhide_descendants_by_filter(api_url, endpoint, filter, db_url=None): +def unhide_descendants_by_filter( + api_url, endpoint, filter, db_url=None, dry_run=False +): """ Be aware that this and hide_descendants_by_filter are not symmetrical. @@ -458,14 +466,21 @@ def unhide_descendants_by_filter(api_url, endpoint, filter, db_url=None): change. """ desc = find_descendants_by_filter( - api_url, endpoint, filter, True, db_url=db_url + api_url, + endpoint, + filter, + ignore_gfs_with_hidden_external_contribs=True, + kfids_only=False, + db_url=db_url, ) - for v in desc.values(): - unhide_kfids(api_url, v) + changed = [] + for es in desc.values(): + changed.extend(unhide_entities(api_url, es.values(), dry_run)) + return changed def hide_descendants_by_kfids( - api_url, endpoint, kfids, gf_acl=None, db_url=None + api_url, endpoint, kfids, gf_acl=None, db_url=None, dry_run=False ): """ Be aware that this and unhide_descendants_by_kfids are not symmetrical. @@ -475,12 +490,22 @@ def hide_descendants_by_kfids( If you anticipate needing symmetrical behavior, keep a record of what you change. """ - desc = find_descendants_by_kfids(db_url or api_url, endpoint, kfids, False) - for v in desc.values(): - hide_kfids(api_url, v, gf_acl) + desc = find_descendants_by_kfids( + db_url or api_url, + endpoint, + kfids, + ignore_gfs_with_hidden_external_contribs=False, + kfids_only=False, + ) + changed = [] + for es in desc.values(): + changed.extend(hide_entities(api_url, es.values(), gf_acl, dry_run)) + return changed -def unhide_descendants_by_kfids(api_url, endpoint, kfids, db_url=None): +def unhide_descendants_by_kfids( + api_url, endpoint, kfids, db_url=None, dry_run=False +): """ Be aware that this and hide_descendants_by_kfids are not symmetrical. @@ -489,6 +514,14 @@ def unhide_descendants_by_kfids(api_url, endpoint, kfids, db_url=None): If you anticipate needing symmetrical behavior, keep a record of what you change. """ - desc = find_descendants_by_kfids(db_url or api_url, endpoint, kfids, True) - for v in desc.values(): - unhide_kfids(api_url, v) + desc = find_descendants_by_kfids( + db_url or api_url, + endpoint, + kfids, + ignore_gfs_with_hidden_external_contribs=True, + kfids_only=False, + ) + changed = [] + for es in desc.values(): + changed.extend(unhide_entities(api_url, es.values(), dry_run)) + return changed diff --git a/tests/integration/test_dataservice_descendants.py b/tests/integration/test_dataservice_descendants.py index bbeaacc..0a8bd9c 100644 --- a/tests/integration/test_dataservice_descendants.py +++ b/tests/integration/test_dataservice_descendants.py @@ -1,9 +1,9 @@ -from kf_utils.dataservice.descendants import find_descendants_by_kfids, find_descendants_by_filter +from kf_utils.dataservice.descendants import find_descendants_by_kfids, find_descendants_by_filter, unhide_descendants_by_kfids, hide_descendants_by_kfids from tests.conftest import DATASERVICE_URL, populate_data def test_by_kfids_simple(dataservice_setup): - n = 4 + n = 2 populate_data(n) desc = find_descendants_by_kfids( @@ -18,7 +18,7 @@ def test_by_kfids_simple(dataservice_setup): desc = find_descendants_by_kfids( DATASERVICE_URL, "participants", [ - "PT_10111111", "PT_11111111", "PT_12111111", "PT_13111111" + "PT_10111111", "PT_11111111" ], ignore_gfs_with_hidden_external_contribs=False, kfids_only=False @@ -36,7 +36,7 @@ def test_by_kfids_simple(dataservice_setup): def test_by_filter_simple(dataservice_setup): - n = 4 + n = 2 populate_data(n) desc = find_descendants_by_filter( DATASERVICE_URL, "participants", {"study_id": "SD_11111111"}, @@ -47,3 +47,18 @@ def test_by_filter_simple(dataservice_setup): assert len(desc["biospecimens"]) == n*n # parent level should be populated assert isinstance(desc["participants"]["PT_11111111"], dict) + + +def test_already_done(dataservice_setup): + n = 2 + all_kfids = populate_data(n) + sdids = ["SD_11111111"] + + changed = unhide_descendants_by_kfids(DATASERVICE_URL, "studies", sdids) + assert not changed + + changed = hide_descendants_by_kfids(DATASERVICE_URL, "studies", sdids) + assert len(changed) == 1 + n + n*n + + changed = unhide_descendants_by_kfids(DATASERVICE_URL, "studies", sdids) + assert len(changed) == 1 + n + n*n