diff --git a/dandi/cli/cmd_upload.py b/dandi/cli/cmd_upload.py index 9b5f1760c..772a05a30 100644 --- a/dandi/cli/cmd_upload.py +++ b/dandi/cli/cmd_upload.py @@ -11,6 +11,7 @@ map_to_click_exceptions, ) from ..consts import SyncMode +from ..exceptions import UploadValidationError from ..upload import UploadExisting, UploadValidation @@ -119,16 +120,19 @@ def upload( validation_companion_path(ctx.obj.logfile) if ctx.obj is not None else None ) - upload_( - paths, - existing=existing, - validation=validation, - dandi_instance=dandi_instance, - allow_any_path=allow_any_path, - upload_dandiset_metadata=upload_dandiset_metadata, - devel_debug=devel_debug, - jobs=jobs, - jobs_per_file=jobs_per_file, - sync=SyncMode(sync) if sync is not None else None, - validation_log_path=companion, - ) + try: + upload_( + paths, + existing=existing, + validation=validation, + dandi_instance=dandi_instance, + allow_any_path=allow_any_path, + upload_dandiset_metadata=upload_dandiset_metadata, + devel_debug=devel_debug, + jobs=jobs, + jobs_per_file=jobs_per_file, + sync=SyncMode(sync) if sync is not None else None, + validation_log_path=companion, + ) + except UploadValidationError as exc: + raise click.ClickException(str(exc)) diff --git a/dandi/cli/tests/test_cmd_upload.py b/dandi/cli/tests/test_cmd_upload.py new file mode 100644 index 000000000..dc6ff9c9d --- /dev/null +++ b/dandi/cli/tests/test_cmd_upload.py @@ -0,0 +1,20 @@ +from click.testing import CliRunner +import pytest +from pytest_mock import MockerFixture + +from ..cmd_upload import upload +from ...exceptions import UploadValidationError + + +@pytest.mark.ai_generated +def test_upload_validation_error_has_no_traceback(mocker: MockerFixture) -> None: + mocker.patch( + "dandi.upload.upload", + side_effect=UploadValidationError("failed validation"), + ) + + result = CliRunner().invoke(upload) + + assert result.exit_code == 1 + assert result.output == "Error: failed validation\n" + assert "Traceback" not in result.output diff --git a/dandi/exceptions.py b/dandi/exceptions.py index fc8639dff..f323d2a42 100644 --- a/dandi/exceptions.py +++ b/dandi/exceptions.py @@ -91,3 +91,9 @@ class HTTP404Error(requests.HTTPError): class UploadError(Exception): pass + + +class UploadValidationError(UploadError): + """An upload could not proceed because an asset failed validation.""" + + pass diff --git a/dandi/tests/fixtures.py b/dandi/tests/fixtures.py index 4c4bb11a8..485cf36e6 100644 --- a/dandi/tests/fixtures.py +++ b/dandi/tests/fixtures.py @@ -592,13 +592,18 @@ class SampleDandiset: def client(self) -> DandiAPIClient: return self.api.client - def upload(self, paths: list[str | Path] | None = None, **kwargs: Any) -> None: + def upload( + self, + paths: list[str | Path] | None = None, + devel_debug: bool = True, + **kwargs: Any, + ) -> None: with pytest.MonkeyPatch().context() as m: self.api.monkeypatch_set_api_key_env(m) upload( paths=paths or [self.dspath], dandi_instance=self.api.instance_id, - devel_debug=True, + devel_debug=devel_debug, **{**self.upload_kwargs, **kwargs}, ) diff --git a/dandi/tests/test_upload.py b/dandi/tests/test_upload.py index cda476474..7f90768a6 100644 --- a/dandi/tests/test_upload.py +++ b/dandi/tests/test_upload.py @@ -207,13 +207,26 @@ def test_upload_sync_do(mocker: MockerFixture, text_dandiset: SampleDandiset) -> text_dandiset.dandiset.get_asset_by_path("file.txt") +@pytest.mark.ai_generated def test_upload_bids_invalid( - mocker: MockerFixture, bids_dandiset_invalid: SampleDandiset + caplog: pytest.LogCaptureFixture, + mocker: MockerFixture, + bids_dandiset_invalid: SampleDandiset, + tmp_path: Path, ) -> None: iter_upload_spy = mocker.spy(LocalFileAsset, "iter_upload") + validation_log = tmp_path / "upload_validation.jsonl" with pytest.raises(UploadError): - bids_dandiset_invalid.upload(existing=UploadExisting.FORCE) + bids_dandiset_invalid.upload( + existing=UploadExisting.FORCE, + validation_log_path=validation_log, + devel_debug=False, + ) iter_upload_spy.assert_not_called() + assert ( + f"Use `dandi validate --load {validation_log}` to review the saved results." + in caplog.text + ) # Does validation ignoring work? bids_dandiset_invalid.upload( existing=UploadExisting.FORCE, validation=UploadValidation.IGNORE diff --git a/dandi/upload.py b/dandi/upload.py index e9d5b2402..2af271d30 100644 --- a/dandi/upload.py +++ b/dandi/upload.py @@ -38,7 +38,7 @@ ) from .dandiapi import DandiAPIClient, RemoteAsset from .dandiset import Dandiset -from .exceptions import NotFoundError, UploadError +from .exceptions import NotFoundError, UploadError, UploadValidationError from .files import ( DandiFile, DandisetMetadataFile, @@ -318,7 +318,7 @@ def process_path(dfile: DandiFile) -> Iterator[dict]: for i, e in enumerate(validation_errors, start=1): lgr.warning(" Error %d: %s", i, e) validate_ok = False - raise UploadError("failed validation") + raise UploadValidationError("failed validation") else: yield {"status": "validated"} else: @@ -478,10 +478,13 @@ def upload_agg(*ignored: Any) -> str: out(rec) if not validate_ok: - lgr.warning( - "One or more assets failed validation. Consult the logfile for" - " details." - ) + msg = "One or more assets failed validation." + if validation_log_path is not None: + msg += ( + f" Use `dandi validate --load {validation_log_path}`" + " to review the saved results." + ) + lgr.warning(msg) if upload_err is not None: try: import etelemetry