-
Notifications
You must be signed in to change notification settings - Fork 0
✨ Delete dataservice entities #24
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
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
811a9c3
:heavy_plus_sign: Add pytest-mock dep
znatty22 8b61431
:construction_worker: Enable tests in circleci
znatty22 292a9c0
:sparkles: Delete entities by study
znatty22 7aa4af5
:white_check_mark: Unit and integration tests for delete ops
znatty22 fa183d6
:memo: Add delete_entities examples to README
znatty22 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,3 @@ | ||
| -r requirements.txt | ||
| pytest | ||
| pytest-mock |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,148 @@ | ||
| import logging | ||
| from pprint import pformat | ||
| from urllib.parse import urlparse | ||
|
|
||
| import requests | ||
| from d3b_utils.requests_retry import Session | ||
|
|
||
| from kf_utils.dataservice.scrape import yield_kfids | ||
|
|
||
| # DO NOT RE-ORDER - deletion requires this order | ||
| ENDPOINTS = [ | ||
| "read-groups", | ||
| "read-group-genomic-files", | ||
| "sequencing-experiments", | ||
| "sequencing-experiment-genomic-files", | ||
| "genomic-files", | ||
| "biospecimen-genomic-files", | ||
| "biospecimens", | ||
| "outcomes", | ||
| "phenotypes", | ||
| "diagnoses", | ||
| "participants", | ||
| "family-relationships", | ||
| "families", | ||
| ] | ||
| STUDIES = "studies" | ||
| LOCAL_HOSTS = { | ||
| "localhost", | ||
| "127.0.0.1", | ||
| } | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| def safe_delete(url, safety_check=True, session=None, **kwargs): | ||
| """ | ||
| Only delete a non-local resource if safety_check is False. | ||
|
|
||
| :param url: url of resource that will be deleted | ||
| :type url: url | ||
| :param saftey_check: Whether to delete if resource is not at localhost | ||
| :type safety_check: bool | ||
| :param session: requests session object to use when deleting | ||
| :type session: requests.Session | ||
| :param kwargs: Keyword args passed to requests.Session.delete | ||
| :type kwarrgs: dict | ||
|
|
||
| :returns: requests.Response from the delete | ||
| :raises: Exception if safety_check=True and url is non-local | ||
| """ | ||
| host = urlparse(url).netloc.split(":")[0] | ||
| if safety_check and (host not in LOCAL_HOSTS): | ||
| raise Exception( | ||
| f"Cannot delete {url}. Safe delete is ENABLED. Resources that are " | ||
| "not on localhost will not be deleted. You can try again with " | ||
| "safe delete disabled." | ||
| ) | ||
|
|
||
| session = session or Session() | ||
| resp = session.delete(url, **kwargs) | ||
| return resp | ||
|
|
||
|
|
||
| def delete_kfids(host, endpoint, kfids, safety_check=True): | ||
| """ | ||
| Delete entities by KF ID. Default behavior only deletes resources at | ||
| localhost unless safety_check=False | ||
|
|
||
| :param host: URL of the Data Service | ||
| :type host: str | ||
| :param endpoint: Data Service endpoint | ||
| :type endpoint: str | ||
| :param kfids: Data Service Kids First IDs | ||
| :type kfids: iterable of strs | ||
| :param saftey_check: Whether to delete if resource is not at localhost | ||
| :type safety_check: bool | ||
|
|
||
| :returns: Dict of errors where Keys are urls that failed delete (non 200 | ||
| status code) and values are requests.Response objects | ||
| """ | ||
| session = Session() | ||
| errors = {} | ||
| kf_ids = [kfid for kfid in kfids] | ||
| total = len(kf_ids) | ||
| for i, kf_id in enumerate(kf_ids): | ||
| url = f"{host}/{endpoint.strip('/')}/{kf_id}" | ||
| logger.info(f"Deleting {i+1} of {total}: {url}") | ||
| resp = safe_delete(url, session=session, safety_check=safety_check) | ||
| try: | ||
| resp.raise_for_status() | ||
| except requests.exceptions.HTTPError: | ||
| logger.error( | ||
| f"Failed to delete {url}, status code {resp.status_code}. " | ||
| f"Response:\n{resp.text}" | ||
| ) | ||
| errors[url] = resp | ||
| return errors | ||
|
|
||
|
|
||
| def delete_entities(host, study_ids=None, safety_check=True): | ||
| """ | ||
| Delete entities by study or delete all entities in Data Service. If | ||
| study_ids is not provided, all entities in Data Service will be deleted. | ||
| Default behavior only deletes resources at localhost unless | ||
| safety_check=False | ||
|
|
||
| Deletion is implemented in a way that avoids large cascading deletions in | ||
| the Data Service database. Large cascading deletes are known to crash | ||
| the Data Service. For example, first we delete genomic files, then | ||
| biospecimens, and then participants rather than deleting participants | ||
| since that would cause a cascading delete of the specimens and their | ||
| genomic files. The order in which entities are deleted is defined in | ||
| ENDPOINTS. | ||
|
|
||
| :param host: URL of the Data Service | ||
| :type host: str | ||
| :param study_ids: If provided, the entities linked to these study ids | ||
| will be deleted, otherwise all entities in Data Service will be deleted | ||
| :type study_ids: list of str | ||
| :param saftey_check: Whether to delete if resource is not at localhost | ||
| :type safety_check: bool | ||
|
|
||
| :returns: Dict of errors where Keys are urls that failed delete (non 200 | ||
| status code) and values are requests.Response objects | ||
| """ | ||
| phrase = f"studies {pformat(study_ids)}" if study_ids else "all studies" | ||
| logger.info(f"Deleting {phrase} from {host}") | ||
|
|
||
| # Get all study ids | ||
| if not study_ids: | ||
| study_ids = yield_kfids(host, "studies", {}) | ||
|
|
||
| # Delete entities by study id | ||
| errors = {} | ||
| for study_id in study_ids: | ||
| # Delete entities except "study" (it has to be handled differently) | ||
| for endpoint in ENDPOINTS: | ||
| params = {"study_id": study_id} | ||
| kfids = yield_kfids(host, endpoint, params) | ||
| errors.update( | ||
| delete_kfids(host, endpoint, kfids, safety_check=safety_check) | ||
| ) | ||
| # Delete study by its kfid | ||
| errors.update( | ||
| delete_kfids(host, STUDIES, [study_id], safety_check=safety_check) | ||
| ) | ||
|
|
||
| return errors |
Empty file.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| import pytest | ||
| from kf_utils.dataservice.delete import delete_entities | ||
|
|
||
| DATASERVICE_URL="http://localhost:5000" | ||
|
|
||
| @pytest.fixture | ||
| def dataservice_setup(): | ||
| """ | ||
| Delete all data in Data Service before and after tests | ||
| """ | ||
| delete_entities(DATASERVICE_URL) | ||
| yield | ||
| delete_entities(DATASERVICE_URL) | ||
|
|
||
Empty file.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| from tests.conftest import DATASERVICE_URL | ||
|
|
||
| import requests | ||
| from kf_utils.dataservice.delete import delete_entities, ENDPOINTS, STUDIES | ||
|
|
||
|
|
||
| def test_delete_entities(dataservice_setup): | ||
| """ | ||
| Test kf_utils.dataservice.delete.delete_entities against live Data Service | ||
| """ | ||
| # Create some data in dataservice | ||
| n_studies = 4 | ||
| n_participants = 2 | ||
| data = { | ||
| "studies": [ | ||
| {"external_id": f"study_{i}", "kf_id": f"SD_{i}1111111"} | ||
| for i in range(n_studies) | ||
| ], | ||
| "participants": [ | ||
| {"kf_id": f"PT_{i}{j}111111", "gender": "Female", | ||
| "study_id": f"SD_{i}1111111"} | ||
| for i in range(n_studies) | ||
| for j in range(n_participants) | ||
| ] | ||
| } | ||
| for endpoint, payloads in data.items(): | ||
| for p in payloads: | ||
| resp = requests.post( | ||
| f"{DATASERVICE_URL}/{endpoint}", json=p | ||
| ) | ||
|
|
||
| # Delete first two studies | ||
| sids = [s["kf_id"] for s in data["studies"][0:2]] | ||
| delete_entities(DATASERVICE_URL, study_ids=sids) | ||
|
|
||
| # Check first two studies deleted, other studies remain | ||
| for i in range(n_studies): | ||
| kfid = data["studies"][i]["kf_id"] | ||
| params = {"study_id": kfid} | ||
| study_resp = requests.get(f"{DATASERVICE_URL}/studies/{kfid}") | ||
| part_resp = requests.get( | ||
| f"{DATASERVICE_URL}/participants", params=params | ||
| ) | ||
| if i <= 1: | ||
| assert study_resp.status_code == 404 | ||
| assert part_resp.json()["total"] == 0 | ||
| else: | ||
| assert study_resp.status_code == 200 | ||
| assert part_resp.json()["total"] == n_participants | ||
|
|
||
| # Delete all studies | ||
| delete_entities(DATASERVICE_URL) | ||
|
|
||
| # Check all study entities deleted | ||
| for endpoint in ENDPOINTS + [STUDIES]: | ||
| resp = requests.get(f"{DATASERVICE_URL}/{endpoint}") | ||
| assert resp.json()["total"] == 0 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,131 @@ | ||
| import pytest | ||
| from unittest.mock import MagicMock, call | ||
|
|
||
| from requests.exceptions import HTTPError | ||
|
|
||
| from kf_utils.dataservice.delete import ( | ||
| delete_kfids, | ||
| delete_entities, | ||
| safe_delete, | ||
| ENDPOINTS, | ||
| STUDIES, | ||
| ) | ||
|
|
||
| HOST = "http://localhost:5000" | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "url,safety_check,mysession,kwargs", | ||
| [ | ||
| ("http://localhost", True, MagicMock(), {"foo": "bar"}), | ||
| ("http://localhost", False, None, {"foo": "bar"}), | ||
| ("http://127.0.0.1", True, MagicMock(), {"foo": "bar"}), | ||
| ("http://127.0.0.1", False, None, {"foo": "bar"}), | ||
| ("http://prd.dataservice.org", True, MagicMock(), {"foo": "bar"}), | ||
| ("http://prd.dataservice.org", False, None, {"foo": "bar"}), | ||
| ("http://10.10.1.191", True, MagicMock(), {"foo": "bar"}), | ||
| ("http://10.10.1.191", False, None, {"foo": "bar"}), | ||
| ] | ||
| ) | ||
| def test_safe_delete(mocker, url, safety_check, mysession, kwargs): | ||
| """ | ||
| Test kf_utils.dataservice.delete.safe_delete | ||
| """ | ||
| # Setup mocks | ||
| mock_session = mocker.patch( | ||
| "kf_utils.dataservice.delete.Session" | ||
| )() | ||
|
|
||
| if safety_check and ("localhost" not in url): | ||
| with pytest.raises(Exception) as e: | ||
| safe_delete( | ||
| url, safety_check=safety_check, session=session | ||
| ) | ||
| assert "Safe delete is ENABLED" in str(e) | ||
| else: | ||
| safe_delete( | ||
| url, safety_check=safety_check, session=mysession, **kwargs | ||
| ) | ||
| if mysession: | ||
| mock_session = mysession | ||
| mock_session.delete.assert_called_with(url, **kwargs) | ||
|
|
||
|
|
||
| def test_delete_kfids(mocker): | ||
| """ | ||
| Test kf_utils.dataservice.delete.delete_kfids | ||
| """ | ||
| # Setup mocks | ||
| mock_session = mocker.patch( | ||
| "kf_utils.dataservice.delete.Session" | ||
| )() | ||
| mock_resp = MagicMock() | ||
| mock_session.delete.return_value = mock_resp | ||
| kfids = [f"kfid{i}" for i in range(2)] | ||
|
|
||
| # Successful delete | ||
| errors = delete_kfids(HOST, "endpoint", kfids) | ||
| assert not errors | ||
| assert mock_session.delete.call_count == len(kfids) | ||
| mock_session.reset_mock() | ||
| mock_resp.reset_mock() | ||
|
|
||
| # Delete with errors | ||
| mock_resp.raise_for_status.side_effect = HTTPError | ||
| mock_session.delete.return_value = mock_resp | ||
| errors = delete_kfids(HOST, "endpoint", kfids) | ||
| assert errors | ||
| for kfid in kfids: | ||
| assert f"{HOST}/endpoint/{kfid}" in errors | ||
| assert mock_session.delete.call_count == len(kfids) | ||
|
|
||
|
|
||
| def test_delete_entities(mocker): | ||
| """ | ||
| Test kf_utils.dataservice.delete.delete_entities | ||
| """ | ||
| mock_yield_kfids = mocker.patch( | ||
| "kf_utils.dataservice.delete.yield_kfids" | ||
| ) | ||
| mock_delete_kfids = mocker.patch( | ||
| "kf_utils.dataservice.delete.delete_kfids" | ||
| ) | ||
| kfids = [f"kfid{i}" for i in range(2)] | ||
| study_ids = [f"study{i}" for i in range(2)] | ||
| mock_yield_kfids.return_value = kfids | ||
| mock_delete_kfids.return_value = {} | ||
|
|
||
| # Delete entities in multiple studies | ||
| errors = delete_entities(HOST, study_ids=study_ids) | ||
| assert not errors | ||
| mock_delete_kfids.assert_has_calls( | ||
| [call(HOST, STUDIES, [sid], safety_check=True) | ||
| for sid in study_ids], | ||
| any_order=True | ||
| ) | ||
| mock_yield_kfids.assert_has_calls( | ||
| [call(HOST, e, {"study_id": sid}) | ||
| for e in ENDPOINTS for sid in study_ids], | ||
| any_order=True | ||
| ) | ||
| mock_yield_kfids.reset_mock() | ||
| mock_delete_kfids.reset_mock() | ||
|
|
||
| # Delete all entities | ||
| mock_yield_kfids.return_value = study_ids | ||
| errors = delete_entities(HOST, study_ids=None) | ||
| assert not errors | ||
| mock_yield_kfids.assert_has_calls( | ||
| [call(HOST, STUDIES, {})] + | ||
| [call(HOST, e, {"study_id": sid}) | ||
| for e in ENDPOINTS for sid in study_ids], | ||
| any_order=True | ||
| ) | ||
| mock_yield_kfids.reset_mock() | ||
| mock_delete_kfids.reset_mock() | ||
|
|
||
| # Delete all entities with error | ||
| mock_errors = {"{HOST}/endpoint/{kfid}": "fake resp" for kfid in kfids} | ||
| mock_delete_kfids.return_value = mock_errors | ||
| errors = delete_entities(HOST, study_ids=study_ids) | ||
| assert errors == mock_errors |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this seems silly for testing delete_entities since this fixture uses the thing we're going to test lol, but we need some way to cleanup the dataservice for all of the integration tests that would make use of the live dataservice. That's also why I added unit tests for delete_entities that make use of mocks and don't depend on the live dataservice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lol no I love it