BF: guard download_generator to not propagate errors

dandi/download.py

@@ -53,6 +53,7 @@
 from .support.digests import get_digest, get_zarr_checksum
 from .support.iterators import IteratorWithAggregation
 from .support.pyout import naturalsize
+from .support.typing import Literal
 from .utils import (
     abbrev_prompt,
     ensure_datetime,
@@ -130,6 +131,7 @@ def download(
         get_metadata=get_metadata,
         get_assets=get_assets,
         jobs_per_zarr=jobs_per_zarr,
+        on_error="yield" if format == "pyout" else "raise",
         **kw,
     )
 
@@ -207,6 +209,7 @@ def download_generator(
     get_metadata: bool = True,
     get_assets: bool = True,
     jobs_per_zarr: Optional[int] = None,
+    on_error: Literal["raise", "yield"] = "raise",
 ) -> Iterator[dict]:
     """A generator for downloads of files, folders, or entire dandiset from DANDI
     (as identified by URL)
@@ -309,13 +312,31 @@ def download_generator(
                     lock=lock,
                 )
 
+            # If exception is raised we might just raise it, or yield
+            # an error record
+            gen = {
+                "raise": _download_generator,
+                "yield": _download_generator_guard(path, _download_generator),
+            }[on_error]
+
             if yield_generator_for_fields:
-                yield {"path": path, yield_generator_for_fields: _download_generator}
+                yield {"path": path, yield_generator_for_fields: gen}
             else:
-                for resp in _download_generator:
+                for resp in gen:
                     yield dict(resp, path=path)
 
 
+def _download_generator_guard(path: str, generator: Iterator[dict]) -> Iterator[dict]:
+    try:
+        yield from generator
+    except Exception as exc:
+        lgr.exception("Caught while downloading %s:", path)
+        yield {
+            "status": "error",
+            "message": str(exc.__class__.__name__),
+        }
+
+
 class ItemsSummary:
     """A helper "structure" to accumulate information about assets to be downloaded
 

dandi/metadata.py

@@ -7,7 +7,6 @@
 import os.path as op
 from pathlib import Path
 import re
-import sys
 from typing import (
     TYPE_CHECKING,
     Any,
@@ -664,10 +663,7 @@ def extract_field(field: str, metadata: dict) -> Any:
 
 
 if TYPE_CHECKING:
-    if sys.version_info >= (3, 8):
-        from typing import TypedDict
-    else:
-        from typing_extensions import TypedDict
+    from .support.typing import TypedDict
 
     class Neurodatum(TypedDict):
         module: str

dandi/support/typing.py

@@ -0,0 +1,6 @@
+import sys
+
+if sys.version_info >= (3, 8):
+    from typing import Literal, TypedDict  # noqa: F401
+else:
+    from typing_extensions import Literal, TypedDict  # noqa: F401

dandi/tests/fixtures.py

@@ -8,7 +8,6 @@
 import re
 import shutil
 from subprocess import DEVNULL, check_output, run
-import sys
 import tempfile
 from time import sleep
 from typing import TYPE_CHECKING, Any, Callable, Dict, Iterator, List, Optional, Union
@@ -179,10 +178,7 @@ def organized_nwb_dir2(
 
 
 if TYPE_CHECKING:
-    if sys.version_info >= (3, 8):
-        from typing import Literal
-    else:
-        from typing_extensions import Literal
+    from ..support.typing import Literal
 
     Scope = Union[
         Literal["session"],
@@ -448,7 +444,9 @@ def bids_dandiset(new_dandiset: SampleDandiset, bids_examples: str) -> SampleDan
 
 
 @pytest.fixture()
-def bids_dandiset_invalid(new_dandiset: SampleDandiset, bids_examples: str) -> SampleDandiset:
+def bids_dandiset_invalid(
+    new_dandiset: SampleDandiset, bids_examples: str
+) -> SampleDandiset:
     copytree(
         os.path.join(bids_examples, "invalid_pet001"),
         str(new_dandiset.dspath) + "/",

dandi/tests/test_download.py

@@ -32,7 +32,9 @@
         "https://dandiarchive.org/dandiset/000027/draft",
     ],
 )
-def test_download_000027(url: str, tmp_path: Path) -> None:
+def test_download_000027(
+    url: str, tmp_path: Path, capsys: pytest.CaptureFixture
+) -> None:
     ret = download(url, tmp_path)  # type: ignore[func-returns-value]
     assert not ret  # we return nothing ATM, might want to "generate"
     dsdir = tmp_path / "000027"
@@ -48,9 +50,14 @@ def test_download_000027(url: str, tmp_path: Path) -> None:
         Digester(["md5"])(dsdir / "sub-RAT123" / "sub-RAT123.nwb")["md5"]
         == "33318fd510094e4304868b4a481d4a5a"
     )
-    # redownload - since already exist there should be an exception
+    # redownload - since already exist there should be an exception if we are
+    # not using pyout
     with pytest.raises(FileExistsError):
-        download(url, tmp_path)
+        download(url, tmp_path, format="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)
+    assert "FileExistsError" in capsys.readouterr().out
 
     # TODO: somehow get that status report about what was downloaded and what not
     download(url, tmp_path, existing="skip")  # TODO: check that skipped

dandi/upload.py

@@ -3,7 +3,6 @@
 import os.path
 from pathlib import Path
 import re
-import sys
 import time
 from typing import TYPE_CHECKING, Any, Dict, Iterator, List, Optional, Set, Tuple, Union
 
@@ -26,10 +25,7 @@
 from .utils import ensure_datetime, get_instance, pluralize
 
 if TYPE_CHECKING:
-    if sys.version_info >= (3, 8):
-        from typing import TypedDict
-    else:
-        from typing_extensions import TypedDict
+    from .support.typing import TypedDict
 
     class Uploaded(TypedDict):
         size: int