From 4df5c026727e75b6ec5dfe9a8cd19433407bd593 Mon Sep 17 00:00:00 2001 From: "John T. Wodder II" Date: Fri, 23 Apr 2021 09:35:12 -0400 Subject: [PATCH] Give "delete" a --skip-missing option --- dandi/cli/cmd_delete.py | 10 ++++- dandi/delete.py | 47 ++++++++++++++++++----- dandi/tests/test_delete.py | 76 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 122 insertions(+), 11 deletions(-) diff --git a/dandi/cli/cmd_delete.py b/dandi/cli/cmd_delete.py index 7ba7a7c28..431ef8203 100644 --- a/dandi/cli/cmd_delete.py +++ b/dandi/cli/cmd_delete.py @@ -4,12 +4,18 @@ @click.command() +@click.option("--skip-missing", is_flag=True) @click.argument("paths", nargs=-1, type=click.Path(exists=False, dir_okay=True)) @instance_option() @devel_debug_option() @map_to_click_exceptions -def delete(paths, dandi_instance="dandi", devel_debug=False): +def delete(paths, skip_missing, dandi_instance="dandi", devel_debug=False): """ Delete Dandisets and assets from the server """ from ..delete import delete - delete(paths, dandi_instance=dandi_instance, devel_debug=devel_debug) + delete( + paths, + dandi_instance=dandi_instance, + devel_debug=devel_debug, + skip_missing=skip_missing, + ) diff --git a/dandi/delete.py b/dandi/delete.py index 22beec21f..f0db702a7 100644 --- a/dandi/delete.py +++ b/dandi/delete.py @@ -30,6 +30,7 @@ class Deleter: dandiset_id: Optional[str] = None #: Whether we are deleting an entire Dandiset (true) or just assets (false) deleting_dandiset: bool = False + skip_missing: bool = False remote_assets: Set[RemoteAsset] = field(default_factory=set) def __bool__(self) -> bool: @@ -55,18 +56,33 @@ def set_dandiset(self, api_url: str, dandiset_id: str) -> None: raise ValueError("Cannot delete assets from multiple Dandisets at once") def register_dandiset(self, api_url: str, dandiset_id: str) -> None: - self.set_dandiset(api_url, dandiset_id) + try: + self.set_dandiset(api_url, dandiset_id) + except NotFoundError: + if self.skip_missing: + return + else: + raise self.deleting_dandiset = True def register_asset( self, api_url: str, dandiset_id: str, version_id: str, asset_path: str ) -> None: - self.set_dandiset(api_url, dandiset_id) + try: + self.set_dandiset(api_url, dandiset_id) + except NotFoundError: + if self.skip_missing: + return + else: + raise asset = self.client.get_asset_bypath(dandiset_id, version_id, asset_path) if asset is None: - raise NotFoundError( - f"Asset at path {asset_path!r} not found in Dandiset {dandiset_id}" - ) + if self.skip_missing: + return + else: + raise NotFoundError( + f"Asset at path {asset_path!r} not found in Dandiset {dandiset_id}" + ) self.remote_assets.add( RemoteAsset(dandiset_id, version_id, asset["asset_id"], asset["path"]) ) @@ -74,7 +90,13 @@ def register_asset( def register_asset_folder( self, api_url: str, dandiset_id: str, version_id: str, folder_path: str ) -> None: - self.set_dandiset(api_url, dandiset_id) + try: + self.set_dandiset(api_url, dandiset_id) + except NotFoundError: + if self.skip_missing: + return + else: + raise any_assets = False for asset in self.client.get_dandiset_assets( dandiset_id, version_id, path=folder_path @@ -83,7 +105,7 @@ def register_asset_folder( RemoteAsset(dandiset_id, version_id, asset["asset_id"], asset["path"]) ) any_assets = True - if not any_assets: + if not any_assets and not self.skip_missing: raise NotFoundError( f"No assets under path {folder_path!r} found in Dandiset {dandiset_id}" ) @@ -120,7 +142,13 @@ def register_local_path_equivalent(self, instance_name: str, filepath: str) -> N raise NotImplementedError("Cannot delete assets from Girder instances") api_url = instance.api dandiset_id, asset_path = find_local_asset(filepath) - self.set_dandiset(api_url, dandiset_id) + try: + self.set_dandiset(api_url, dandiset_id) + except NotFoundError: + if self.skip_missing: + return + else: + raise if asset_path.endswith("/"): self.register_asset_folder(api_url, dandiset_id, "draft", asset_path) else: @@ -175,8 +203,9 @@ def delete( devel_debug: bool = False, jobs: Optional[int] = None, force: bool = False, + skip_missing: bool = False, ) -> None: - deleter = Deleter() + deleter = Deleter(skip_missing=skip_missing) for p in paths: if is_url(p): deleter.register_url(p) diff --git a/dandi/tests/test_delete.py b/dandi/tests/test_delete.py index e5ad251ef..90b5db5a4 100644 --- a/dandi/tests/test_delete.py +++ b/dandi/tests/test_delete.py @@ -226,6 +226,20 @@ def test_delete_nonexistent_dandiset(local_dandi_api, mocker, monkeypatch): delete_spy.assert_not_called() +def test_delete_nonexistent_dandiset_skip_missing(local_dandi_api, mocker, monkeypatch): + monkeypatch.setenv("DANDI_API_KEY", local_dandi_api["api_key"]) + instance = local_dandi_api["instance_id"] + delete_spy = mocker.spy(RESTFullAPIClient, "delete") + delete( + [f"dandi://{instance}/999999/subdir1/apple.txt"], + dandi_instance=instance, + devel_debug=True, + force=True, + skip_missing=True, + ) + delete_spy.assert_not_called() + + def test_delete_nonexistent_asset(local_dandi_api, mocker, monkeypatch, text_dandiset): monkeypatch.setenv("DANDI_API_KEY", local_dandi_api["api_key"]) instance = local_dandi_api["instance_id"] @@ -248,6 +262,37 @@ def test_delete_nonexistent_asset(local_dandi_api, mocker, monkeypatch, text_dan delete_spy.assert_not_called() +def test_delete_nonexistent_asset_skip_missing( + local_dandi_api, mocker, monkeypatch, text_dandiset, tmp_path +): + monkeypatch.setenv("DANDI_API_KEY", local_dandi_api["api_key"]) + instance = local_dandi_api["instance_id"] + dandiset_id = text_dandiset["dandiset_id"] + delete_spy = mocker.spy(RESTFullAPIClient, "delete") + delete( + [ + f"dandi://{instance}/{dandiset_id}/file.txt", + f"dandi://{instance}/{dandiset_id}/subdir3/mango.txt", + ], + dandi_instance=instance, + devel_debug=True, + force=True, + skip_missing=True, + ) + delete_spy.assert_called() + download( + f"{local_dandi_api['instance'].api}/dandisets/{dandiset_id}/versions/draft", + tmp_path, + ) + files = sorted(map(Path, find_files(r".*", paths=[tmp_path]))) + assert files == [ + tmp_path / dandiset_id / "dandiset.yaml", + tmp_path / dandiset_id / "subdir1" / "apple.txt", + tmp_path / dandiset_id / "subdir2" / "banana.txt", + tmp_path / dandiset_id / "subdir2" / "coconut.txt", + ] + + def test_delete_nonexistent_asset_folder( local_dandi_api, mocker, monkeypatch, text_dandiset ): @@ -272,6 +317,37 @@ def test_delete_nonexistent_asset_folder( delete_spy.assert_not_called() +def test_delete_nonexistent_asset_folder_skip_missing( + local_dandi_api, mocker, monkeypatch, text_dandiset, tmp_path +): + monkeypatch.setenv("DANDI_API_KEY", local_dandi_api["api_key"]) + instance = local_dandi_api["instance_id"] + dandiset_id = text_dandiset["dandiset_id"] + delete_spy = mocker.spy(RESTFullAPIClient, "delete") + delete( + [ + f"dandi://{instance}/{dandiset_id}/subdir1/", + f"dandi://{instance}/{dandiset_id}/subdir3/", + ], + dandi_instance=instance, + devel_debug=True, + force=True, + skip_missing=True, + ) + delete_spy.assert_called() + download( + f"{local_dandi_api['instance'].api}/dandisets/{dandiset_id}/versions/draft", + tmp_path, + ) + files = sorted(map(Path, find_files(r".*", paths=[tmp_path]))) + assert files == [ + tmp_path / dandiset_id / "dandiset.yaml", + tmp_path / dandiset_id / "file.txt", + tmp_path / dandiset_id / "subdir2" / "banana.txt", + tmp_path / dandiset_id / "subdir2" / "coconut.txt", + ] + + def test_delete_version(local_dandi_api, mocker, monkeypatch): monkeypatch.setenv("DANDI_API_KEY", local_dandi_api["api_key"]) instance = local_dandi_api["instance_id"]