Merge pull request #1536 from dandi/enh-fail-download

Do fail (raise Exception, CLI has non-0 exit) download if any of downloads fail

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

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",