diff --git a/dandi/download.py b/dandi/download.py index 0973d956f..235381420 100644 --- a/dandi/download.py +++ b/dandi/download.py @@ -7,6 +7,7 @@ from enum import Enum from functools import partial import hashlib +import inspect import json import os import os.path as op @@ -153,6 +154,26 @@ def download( gen_ = (r for dl in downloaders for r in dl.download_generator()) + # Constructs to capture errors and handle them at the end + errors = [] + + def p4e_gen(callback): + for v in callback: + yield p4e(v) + + def p4e(out): + if out.get("status") == "error": + if out not in errors: + errors.append(out) + else: + # If generator was yielded, we need to wrap it also with + # our handling + for k, v in out.items(): + if inspect.isgenerator(v): + rec[k] = p4e_gen(v) + + return out + # TODOs: # - redo frontends similarly to how command_ls did it # - have a single loop with analysis of `rec` to either any file @@ -161,11 +182,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}") @@ -191,6 +212,10 @@ def download( break else: break + if errors: + raise RuntimeError( + f"Encountered {pluralize(len(errors), 'error')} while downloading." + ) @dataclass diff --git a/dandi/tests/test_download.py b/dandi/tests/test_download.py index 61d3a9e65..a06d94aec 100644 --- a/dandi/tests/test_download.py +++ b/dandi/tests/test_download.py @@ -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 @@ -70,9 +71,11 @@ 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) as exc: + download(url, tmp_path) assert "FileExistsError" in capsys.readouterr().out + assert "Encountered 1 error while downloading" in str(exc) # TODO: somehow get that status report about what was downloaded and what not download(url, tmp_path, existing=DownloadExisting.SKIP) # TODO: check that skipped @@ -147,7 +150,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",