Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Give "delete" a --skip-missing option #594

Merged
merged 1 commit into from
Apr 26, 2021
Merged

Give "delete" a --skip-missing option #594

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions dandi/cli/cmd_delete.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
47 changes: 38 additions & 9 deletions dandi/delete.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -55,26 +56,47 @@ 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"])
)

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
Expand All @@ -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}"
)
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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)
Expand Down
76 changes: 76 additions & 0 deletions dandi/tests/test_delete.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand All @@ -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
):
Expand All @@ -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"]
Expand Down