Point failed uploads to validation report

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))

dandi/cli/tests/test_cmd_upload.py

@@ -0,0 +1,22 @@
+from click.testing import CliRunner
+import pytest
+from pytest_mock import MockerFixture
+
+from ..base import map_to_click_exceptions
+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.object(map_to_click_exceptions, "_do_map", False)
+    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

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

dandi/tests/test_upload.py

@@ -207,13 +207,25 @@ 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,
+        )
     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

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:
@@ -451,7 +451,18 @@ def upload_agg(*ignored: Any) -> str:
             style=pyout_style, columns=rec_fields, max_workers=jobs or 5
         )
 
-        with out:
+        def report_validation_failure() -> None:
+            if not validate_ok:
+                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)
+
+        with ExitStack() as warning_stack, out:
+            warning_stack.callback(report_validation_failure)
             for dfile in dandi_files:
                 while len(process_paths) >= 10:
                     lgr.log(2, "Sleep waiting for some paths to finish processing")
@@ -476,12 +487,6 @@ def upload_agg(*ignored: Any) -> str:
                 except ValueError as exc:
                     rec.update(error_file(exc))
                 out(rec)
-
-        if not validate_ok:
-            lgr.warning(
-                "One or more assets failed validation.  Consult the logfile for"
-                " details."
-            )
         if upload_err is not None:
             try:
                 import etelemetry