Skip to content

Warn when uploads omit unrecognized paths - #1915

Open
AtomicGlance wants to merge 3 commits into
dandi:masterfrom
AtomicGlance:fix/warn-skipped-upload-paths
Open

Warn when uploads omit unrecognized paths#1915
AtomicGlance wants to merge 3 commits into
dandi:masterfrom
AtomicGlance:fix/warn-skipped-upload-paths

Conversation

@AtomicGlance

Copy link
Copy Markdown
Contributor

Fixes #1493

dandi upload discovers only recognized assets. Before this change, a requested directory could finish with files omitted from the upload without telling the user.

This adds a discovery helper that reports omitted files individually when a requested tree contains recognized assets, or collapses a wholly unrecognized directory to one entry. It keeps Dandiset metadata, dot/VCS paths, common OS metadata, empty directories, Zarr contents, and directory symlinks out of the warning. Partial uploads stay scoped to the requested paths, and --allow-any-path remains an explicit opt-out.

The warning is registered after the progress table is created, so it is emitted after the table closes and still appears when another asset upload fails.

Tests added:

  • mixed recognized and unrecognized trees, unknown directories, ignored paths, Zarr directories, empty directories, and directory symlinks
  • whole-Dandiset warning, partial-upload suppression, --allow-any-path suppression, and warning-on-upload-error behavior

Local validation:

  • focused discovery tests: 3 passed, 1 symlink test skipped because the platform does not permit the fixture
  • upload tests collected successfully; they require the repository's Docker-backed API fixture, and Docker is not installed in this environment
  • ruff check and python -m py_compile passed

@codecov

codecov Bot commented Sep 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 62.18487% with 45 lines in your changes missing coverage. Please review.
✅ Project coverage is 54.30%. Comparing base (d91a041) to head (95ac3d3).

Files with missing lines Patch % Lines
dandi/tests/test_upload.py 12.12% 29 Missing ⚠️
dandi/upload.py 0.00% 8 Missing ⚠️
dandi/files/__init__.py 87.50% 6 Missing ⚠️
dandi/tests/test_files.py 93.33% 2 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (d91a041) and HEAD (95ac3d3). Click for more details.

HEAD has 109 uploads less than BASE
Flag BASE (d91a041) HEAD (95ac3d3)
unittests 124 15
Additional details and impacted files
@@             Coverage Diff             @@
##           master    #1915       +/-   ##
===========================================
- Coverage   77.35%   54.30%   -23.05%     
===========================================
  Files          89       89               
  Lines       13325    13443      +118     
===========================================
- Hits        10307     7300     -3007     
- Misses       3018     6143     +3125     
Flag Coverage Δ
unittests 54.30% <62.18%> (-23.05%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@AtomicGlance

Copy link
Copy Markdown
Contributor Author

Fixed the three Linux upload-test failures by placing the NWB fixture under a valid sub-01/ subject directory. The implementation was not changed. The helper test passes locally; the Docker-backed upload tests are covered by CI. Please rerun the checks when convenient.

@AtomicGlance

Copy link
Copy Markdown
Contributor Author

The rerun found one remaining assertion: the singular warning used “1 path were”. I fixed that in 95ac3d3 so one path reads “was” while plural counts keep “were”. Ruff checks pass; CI is running again.

@AtomicGlance

Copy link
Copy Markdown
Contributor Author

One macOS 3.13 job in the new matrix failed while cloning the external dandi-datasets/nwb_test_data repository (14 setup errors; 749 tests passed). That failure is unrelated to this PR. The upload-warning integration job is still running.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

upload: if any file/path is skipped due to not part of DANDI layout -- log WARNING upon completion or ask

1 participant