Skip to content

Commit

Permalink
RF: download - do raise exception if any download fails
Browse files Browse the repository at this point in the history
Otherwise it is hard-to-impossible to script using "dandi download" reliably
  • Loading branch information
yarikoptic committed Dec 2, 2024
1 parent 5215b37 commit 9611632
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 5 deletions.
13 changes: 11 additions & 2 deletions dandi/download.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,13 @@ def download(

gen_ = (r for dl in downloaders for r in dl.download_generator())

errors = []

def p4e(out):
if out.get("status") == "error":
errors.append(out)
return out

# TODOs:
# - redo frontends similarly to how command_ls did it
# - have a single loop with analysis of `rec` to either any file
Expand All @@ -161,11 +168,11 @@ def download(
#
if format is DownloadFormat.DEBUG:
for rec in gen_:
print(rec, flush=True)
print(p4e(rec), flush=True)
elif format is DownloadFormat.PYOUT:
with out:
for rec in gen_:
out(rec)
out(p4e(rec))
else:
raise AssertionError(f"Unhandled DownloadFormat member: {format!r}")

Expand All @@ -191,6 +198,8 @@ def download(
break
else:
break
if errors:
raise RuntimeError(f"Encountered {len(errors)} errors while downloading.")


@dataclass
Expand Down
9 changes: 6 additions & 3 deletions dandi/tests/test_download.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from __future__ import annotations

from collections.abc import Callable
from contextlib import nullcontext
from glob import glob
import json
import logging
Expand Down Expand Up @@ -70,8 +71,9 @@ def test_download_000027(
with pytest.raises(FileExistsError):
download(url, tmp_path, format=DownloadFormat.DEBUG)
assert "FileExistsError" not in capsys.readouterr().out
# but no exception is raised, and rather it gets output to pyout otherwise
download(url, tmp_path)
# Generic error should be raised if we are using pyout/parallelization
with pytest.raises(RuntimeError):
download(url, tmp_path)
assert "FileExistsError" in capsys.readouterr().out

# TODO: somehow get that status report about what was downloaded and what not
Expand Down Expand Up @@ -147,7 +149,8 @@ def test_download_000027_resume(
with (dldir / "checksum").open("w") as fp:
json.dump(digests, fp)

download(url, tmp_path, get_metadata=False)
with pytest.raises(RuntimeError) if break_download else nullcontext():
download(url, tmp_path, get_metadata=False)
assert list_paths(dsdir, dirs=True) == [
dsdir / "sub-RAT123",
dsdir / "sub-RAT123" / "sub-RAT123.nwb",
Expand Down

0 comments on commit 9611632

Please sign in to comment.