Warn when uploads omit unrecognized paths

dandi/tests/test_upload.py

@@ -314,6 +314,85 @@ def test_upload_bids_non_nwb_file(bids_dandiset: SampleDandiset) -> None:
     assert [asset.path for asset in bids_dandiset.dandiset.get_assets()] == ["README"]
 
 
+@pytest.mark.ai_generated
+def test_upload_warns_for_unrecognized_paths(
+    caplog: pytest.LogCaptureFixture,
+    new_dandiset: SampleDandiset,
+    simple2_nwb: Path,
+) -> None:
+    subject_dir = new_dandiset.dspath / "sub-01"
+    subject_dir.mkdir()
+    copyfile(simple2_nwb, subject_dir / "sub-01.nwb")
+    (new_dandiset.dspath / "sidecar.json").write_text("{}")
+    (new_dandiset.dspath / "notes").mkdir()
+    (new_dandiset.dspath / "notes" / "readme.txt").write_text("notes")
+
+    with caplog.at_level("WARNING", logger="dandi"):
+        new_dandiset.upload()
+
+    (record,) = [
+        r for r in caplog.records if "not recognized as DANDI assets" in r.message
+    ]
+    assert isinstance(record.args, tuple)
+    assert record.args[-1] == "notes, sidecar.json"
+
+
+@pytest.mark.ai_generated
+def test_upload_partial_does_not_warn_for_unrequested_paths(
+    caplog: pytest.LogCaptureFixture,
+    new_dandiset: SampleDandiset,
+    simple2_nwb: Path,
+) -> None:
+    subject_dir = new_dandiset.dspath / "sub-01"
+    subject_dir.mkdir()
+    nwb_path = subject_dir / "sub-01.nwb"
+    copyfile(simple2_nwb, nwb_path)
+    (new_dandiset.dspath / "sidecar.json").write_text("{}")
+
+    with caplog.at_level("WARNING", logger="dandi"):
+        new_dandiset.upload(paths=[nwb_path])
+
+    assert "not recognized as DANDI assets" not in caplog.text
+
+
+@pytest.mark.ai_generated
+def test_upload_allow_any_path_suppresses_omission_warning(
+    caplog: pytest.LogCaptureFixture, text_dandiset: SampleDandiset
+) -> None:
+    with caplog.at_level("WARNING", logger="dandi"):
+        text_dandiset.upload()
+
+    assert "not recognized as DANDI assets" not in caplog.text
+
+
+@pytest.mark.ai_generated
+def test_upload_omission_warning_survives_upload_error(
+    caplog: pytest.LogCaptureFixture,
+    mocker: MockerFixture,
+    new_dandiset: SampleDandiset,
+    simple2_nwb: Path,
+) -> None:
+    subject_dir = new_dandiset.dspath / "sub-01"
+    subject_dir.mkdir()
+    copyfile(simple2_nwb, subject_dir / "sub-01.nwb")
+    (new_dandiset.dspath / "sidecar.json").write_text("{}")
+    mocker.patch.object(
+        LocalFileAsset, "iter_upload", side_effect=UploadError("upload failed")
+    )
+
+    with (
+        caplog.at_level("WARNING", logger="dandi"),
+        pytest.raises(UploadError, match="upload failed"),
+    ):
+        new_dandiset.upload()
+
+    (record,) = [
+        r for r in caplog.records if "not recognized as DANDI assets" in r.message
+    ]
+    assert isinstance(record.args, tuple)
+    assert record.args[-1] == "sidecar.json"
+
+
 @sweep_embargo
 def test_upload_sync_zarr(
     mocker: MockerFixture, zarr_dandiset: SampleDandiset, embargo: bool

dandi/tests/test_upload_discovery.py

@@ -0,0 +1,108 @@
+from pathlib import Path, PurePosixPath
+
+import pytest
+from pytest_mock import MockerFixture
+
+from .test_files import mkpaths
+from ..dandiset import Dandiset
+from ..files import find_dandi_files
+from ..upload import _partition_upload_assets, upload
+
+
+@pytest.mark.ai_generated
+@pytest.mark.parametrize("allow_any_path", [False, True])
+@pytest.mark.parametrize(
+    "roots, expected",
+    [
+        (["."], ["Thumbs.db", "mixed/sidecar.json", "notes", "unknown.txt"]),
+        (["mixed"], ["mixed/sidecar.json"]),
+        (["notes"], ["notes/nested"]),
+        (["notes/nested/readme.txt"], ["notes/nested/readme.txt"]),
+        (["mixed", "mixed/known.nwb"], ["mixed/sidecar.json"]),
+        (["sample.zarr"], []),
+        (["empty", ".hidden"], []),
+    ],
+)
+def test_partition_matches_discovery(
+    tmp_path: Path, roots: list[str], expected: list[str], allow_any_path: bool
+) -> None:
+    mkpaths(
+        tmp_path,
+        "dandiset.yaml",
+        "known.nwb",
+        "unknown.txt",
+        "Thumbs.db",
+        "notes/nested/readme.txt",
+        "mixed/known.nwb",
+        "mixed/sidecar.json",
+        "sample.zarr/chunk",
+        "empty/",
+        ".hidden/secret.nwb",
+        "__MACOSX/._known.nwb",
+    )
+    ds = Dandiset(tmp_path)
+    assets = ds.assets(allow_all=True)
+    selected, omitted = _partition_upload_assets(
+        assets, [PurePosixPath(p) for p in roots], allow_any_path
+    )
+    # Compare against the real discovery pipeline, including request scoping.
+    baseline = ds.assets(allow_all=allow_any_path)
+    assert sorted(a.path for a in selected) == sorted(
+        {a.path for root in roots for a in baseline.under_paths([root])}
+    )
+    assert [str(p) for p in omitted] == ([] if allow_any_path else expected)
+
+
+@pytest.mark.ai_generated
+def test_wholly_unrecognized_dandiset(tmp_path: Path) -> None:
+    mkpaths(tmp_path, "dandiset.yaml", "a.txt", "notes/readme.txt")
+    selected, omitted = _partition_upload_assets(
+        Dandiset(tmp_path).assets(allow_all=True), [PurePosixPath(".")], False
+    )
+    assert selected == []
+    assert omitted == [PurePosixPath("a.txt"), PurePosixPath("notes")]
+
+
+@pytest.mark.ai_generated
+def test_bids_assets_stay_recognized(tmp_path: Path) -> None:
+    mkpaths(tmp_path, "dandiset.yaml", "dataset_description.json", "sidecar.json")
+    assets = Dandiset(tmp_path).assets(allow_all=True)
+    selected, omitted = _partition_upload_assets(assets, [PurePosixPath(".")], False)
+    assert sorted(a.filepath for a in selected) == sorted(
+        a.filepath for a in find_dandi_files(tmp_path, dandiset_path=tmp_path)
+    )
+    assert omitted == []
+
+
+@pytest.mark.ai_generated
+def test_symlinked_directory_is_not_an_omitted_asset(tmp_path: Path) -> None:
+    mkpaths(tmp_path, "dandiset.yaml", "target/notes.txt")
+    try:
+        (tmp_path / "linked").symlink_to(tmp_path / "target", target_is_directory=True)
+    except OSError as exc:
+        pytest.skip(f"Cannot create directory symlink: {exc}")
+    selected, omitted = _partition_upload_assets(
+        Dandiset(tmp_path).assets(allow_all=True), [PurePosixPath("linked")], False
+    )
+    assert selected == []
+    assert omitted == []
+
+
+@pytest.mark.ai_generated
+@pytest.mark.parametrize("count", [1, 12])
+def test_upload_unknown_only_reports_paths(
+    tmp_path: Path, mocker: MockerFixture, caplog: pytest.LogCaptureFixture, count: int
+) -> None:
+    mkpaths(tmp_path, "dandiset.yaml", *(f"note-{i:02}.txt" for i in range(count)))
+    (tmp_path / "dandiset.yaml").write_text("identifier: '000001'\n")
+    mocker.patch("dandi.upload.DandiAPIClient.for_dandi_instance")
+    upload([tmp_path])
+    (record,) = [
+        r for r in caplog.records if "not recognized as DANDI assets" in r.message
+    ]
+    expected = ", ".join(f"note-{i:02}.txt" for i in range(min(count, 10)))
+    if count > 10:
+        expected += ", ..."
+    assert isinstance(record.args, tuple)
+    assert record.args[-1] == expected
+    assert "note-11.txt" in caplog.text if count > 10 else "note-00.txt" in caplog.text

dandi/upload.py

@@ -17,7 +17,7 @@
 from enum import StrEnum
 import io
 import os.path
-from pathlib import Path
+from pathlib import Path, PurePosixPath
 import re
 import time
 from time import sleep
@@ -37,11 +37,12 @@
     dandiset_metadata_file,
 )
 from .dandiapi import DandiAPIClient, RemoteAsset
-from .dandiset import Dandiset
+from .dandiset import AssetView, Dandiset
 from .exceptions import NotFoundError, UploadError, UploadValidationError
 from .files import (
     DandiFile,
     DandisetMetadataFile,
+    GenericAsset,
     LocalAsset,
     LocalDirectoryAsset,
     ZarrAsset,
@@ -54,6 +55,39 @@
 from .validate._types import Severity
 
 
+def _partition_upload_assets(
+    assets: AssetView, roots: Sequence[PurePosixPath], allow_any_path: bool
+) -> tuple[list[LocalAsset], list[PurePosixPath]]:
+    """Select uploads and collapse omitted paths using the existing discovery."""
+    root_set = set(roots)
+    roots = [p for p in root_set if not any(a in root_set for a in p.parents)]
+    selected = []
+    omitted = []
+    for asset in assets.under_paths(roots):
+        if type(asset) is GenericAsset and not allow_any_path:
+            omitted.append(PurePosixPath(asset.path))
+        else:
+            selected.append(asset)
+    keep = {
+        ancestor
+        for asset in selected
+        for ancestor in (PurePosixPath(asset.path), *PurePosixPath(asset.path).parents)
+    }
+    boundaries = set(roots) | {PurePosixPath(".")}
+    collapsed = set()
+    for path in omitted:
+        candidate = path
+        if path in boundaries:
+            collapsed.add(path)
+            continue
+        for ancestor in path.parents:
+            if ancestor in boundaries or ancestor in keep:
+                break
+            candidate = ancestor
+        collapsed.add(candidate)
+    return selected, sorted(collapsed)
+
+
 def _check_dandidownload_paths(dfile: DandiFile) -> None:
     """
     Check if an asset contains .dandidownload paths and raise UploadError if found.
@@ -235,14 +269,17 @@ def new_super_len(o: Any) -> int:
         # DO NOT FACTOR OUT THIS VARIABLE!  It stores any
         # BIDSDatasetDescriptionAsset instances for the Dandiset, which need to
         # remain alive until we're done working with all BIDS assets.
-        assets = dandiset.assets(allow_all=allow_any_path)
+        assets = dandiset.assets(allow_all=True)
+        selected, omitted_paths = _partition_upload_assets(
+            assets,
+            [PurePosixPath(Path(p).relative_to(dandiset.path)) for p in paths],
+            allow_any_path,
+        )
 
         dandi_files: list[DandiFile] = []
         # Build the list step by step so as not to confuse mypy
         dandi_files.append(dandiset.metadata_file())
-        dandi_files.extend(
-            assets.under_paths(Path(p).relative_to(dandiset.path) for p in paths)
-        )
+        dandi_files.extend(selected)
         lgr.info(f"Found {len(dandi_files)} files to consider")
 
         # We will keep a shared set of "being processed" paths so
@@ -461,8 +498,22 @@ def report_validation_failure() -> None:
                     )
                 lgr.warning(msg)
 
+        def report_omitted_paths() -> None:
+            if not omitted_paths:
+                return
+
+            relpaths = [path.as_posix() for path in omitted_paths]
+            lgr.warning(
+                "%s not uploaded (not recognized as DANDI assets): %s. "
+                "Review the paths or use --allow-any-path if intentional.",
+                pluralize(len(relpaths), "path"),
+                ", ".join(relpaths[:10]) + (", ..." if len(relpaths) > 10 else ""),
+            )
+            lgr.debug("Complete list of paths not uploaded: %s", ", ".join(relpaths))
+
         with ExitStack() as warning_stack, out:
             warning_stack.callback(report_validation_failure)
+            warning_stack.callback(report_omitted_paths)
             for dfile in dandi_files:
                 while len(process_paths) >= 10:
                     lgr.log(2, "Sleep waiting for some paths to finish processing")