Skip to content

Commit

Permalink
BF: we need to also wrap callbacks to capture errors in their returns
Browse files Browse the repository at this point in the history
  • Loading branch information
yarikoptic committed Dec 2, 2024
1 parent 9611632 commit 9b3f2fb
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 3 deletions.
20 changes: 18 additions & 2 deletions dandi/download.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -153,11 +154,24 @@ 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)

Check warning on line 162 in dandi/download.py

View check run for this annotation

Codecov / codecov/patch

dandi/download.py#L161-L162

Added lines #L161 - L162 were not covered by tests

def p4e(out):
if out.get("status") == "error":
errors.append(out)
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)

Check warning on line 173 in dandi/download.py

View check run for this annotation

Codecov / codecov/patch

dandi/download.py#L173

Added line #L173 was not covered by tests

return out

# TODOs:
Expand Down Expand Up @@ -199,7 +213,9 @@ def p4e(out):
else:
break
if errors:
raise RuntimeError(f"Encountered {len(errors)} errors while downloading.")
raise RuntimeError(
f"Encountered {pluralize(len(errors), 'error')} while downloading."
)


@dataclass
Expand Down
3 changes: 2 additions & 1 deletion dandi/tests/test_download.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,10 @@ def test_download_000027(
download(url, tmp_path, format=DownloadFormat.DEBUG)
assert "FileExistsError" not in capsys.readouterr().out
# Generic error should be raised if we are using pyout/parallelization
with pytest.raises(RuntimeError):
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
Expand Down

0 comments on commit 9b3f2fb

Please sign in to comment.