Skip to content

Say why metadata failed to load and how to see the tracebacks - #1901

Open
adityasingh2400 wants to merge 1 commit into
dandi:masterfrom
adityasingh2400:fix-1640
Open

Say why metadata failed to load and how to see the tracebacks#1901
adityasingh2400 wants to merge 1 commit into
dandi:masterfrom
adityasingh2400:fix-1640

Conversation

@adityasingh2400

Copy link
Copy Markdown
Contributor

Fixes #1640

dandi organize warned Failed to load metadata for N out of M files due to following types of exceptions: ConstructError. Details of the exceptions will be shown at DEBUG level and stopped there. The user got an exception class name, no reason, and no hint about how to reach DEBUG level or where the log file lives. As the reporter notes, most people running organize are not programmers.

The warning now names each offending path with its exception type and the exception's own message, capped at MAX_METADATA_ERRORS_SHOWN = 5 so a large batch does not flood the console. It also says that full tracebacks are written to the log file, and that dandi --log-level DEBUG organize ... prints them to the console.

The message is built by format_metadata_load_failures() so it can be tested without running a full organize.

Verified against the base ref. On master the integration test fails because the reason never appears in the warnings. On this branch it does. One note on that check: the two unit tests import the new symbols, so a plain revert fails at collection rather than on behavior. To get a genuinely behavioral before signal I reverted organize.py to the base ref and also removed the two new imports from the test file, then ran only the integration test, which fails on the old message content. Both were restored afterward.

New tests are marked @pytest.mark.ai_generated. dandi/tests/test_organize.py excluding integration gives 34 passed, 14 deselected.

AI assistance disclosure: this change was written with the help of Claude Code, and the added tests are marked ai_generated as CLAUDE.md asks. I reviewed and tested everything before submitting.

`dandi organize` warned "Failed to load metadata for N out of M files due
to following types of exceptions: ConstructError. Details of the
exceptions will be shown at DEBUG level" and stopped there. The user got
an exception class name, no reason, and no way to find out how to reach
DEBUG level. Most people running organize are not programmers and do not
know about log levels or where the log file lives.

The warning now names each offending path with its exception type and
the exception's own message, capped at MAX_METADATA_ERRORS_SHOWN so a
large batch does not flood the console, and it says that full tracebacks
are in the log file and that 'dandi --log-level DEBUG organize ...'
prints them to the console.

The message is built by `format_metadata_load_failures()` so it can be
tested without running a full organize.

Closes dandi#1640
@yarikoptic

Copy link
Copy Markdown
Member

The warning now names each offending path with its exception type and the exception's own message, capped at MAX_METADATA_ERRORS_SHOWN = 5 so a large batch does not flood the console

that's where I would have loved to see that new and improved output in comparison to the original quoted. Do you still have a realistic use case handy to demo on ?

@yarikoptic yarikoptic added enhancement New feature or request UX patch Increment the patch version when merged labels Aug 7, 2026
@adityasingh2400

Copy link
Copy Markdown
Contributor Author

Fair ask. Here is the same batch rendered both ways.

The scenario is a mixed run of 8 files: two corrupt NWBs, a non-NWB file that got a .nwb suffix, one unreadable due to permissions, three missing subject_id, and one that loads fine.

Before

Failed to load metadata for 7 out of 8 files due to following types of exceptions: OSError, OSError, TypeError, PermissionError, KeyError, KeyError, KeyError. Details of the exceptions will be shown at DEBUG level

After

Failed to load metadata for 7 out of 8 files:
  sub-01/ses-1_ecephys.nwb: OSError: Unable to open file (file signature not found)
  sub-02/ses-1_ecephys.nwb: OSError: Unable to open file (truncated file: eof)
  notes/readme_renamed.nwb: TypeError: 'NoneType' object is not subscriptable
  sub-03/ses-2_behavior.nwb: PermissionError: [Errno 13] Permission denied
  sub-04/ses-1_ecephys.nwb: KeyError: 'subject_id'
  ... and 2 more, see the log file
Full tracebacks are recorded in the log file whose location is reported at the end of this run.  To also see them on the console, re-run as 'dandi --log-level DEBUG organize ...'.

Three things the old one costs you, visible in that comparison:

  1. No paths. You learn 7 files failed but not which, so you cannot go look at them without turning on DEBUG and re-running the whole organize.
  2. No messages. OSError alone does not distinguish a truncated file from a bad signature from a permissions problem. Those are three different fixes.
  3. It repeats types rather than summarising them. It says "following types of exceptions" but emits one entry per failure, so 7 failures of 4 distinct types print 7 names. At 50 failures that line becomes unreadable, which is the case the MAX_METADATA_ERRORS_SHOWN cap is there for.

One caveat on how I produced this. I rendered both messages directly from the old and new formatting code with the exception tuples above, rather than by corrupting real NWB files, since I do not have a dandiset handy here. The inputs are the shape _get_metadata actually returns, (exception class, exception message, traceback), so the formatting is exercised for real, but the exceptions themselves are constructed rather than raised by hdf5. If you would rather see it end to end on an actual dandiset I am happy to do that instead.

@codecov

codecov Bot commented Aug 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.73684% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.01%. Comparing base (2b5f8ea) to head (ea0c487).
⚠️ Report is 19 commits behind head on master.

Files with missing lines Patch % Lines
dandi/organize.py 80.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1901      +/-   ##
==========================================
+ Coverage   76.96%   77.01%   +0.05%     
==========================================
  Files          88       88              
  Lines       12882    12919      +37     
==========================================
+ Hits         9914     9949      +35     
- Misses       2968     2970       +2     
Flag Coverage Δ
unittests 77.01% <94.73%> (+0.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.

@adityasingh2400

Copy link
Copy Markdown
Contributor Author

Unrelated to the demo, but worth flagging so you do not spend time on it: the red check_labels is stale and cannot go green by re-running.

The workflow reads labels from the event payload rather than querying the API:

const labels = context.payload.pull_request.labels.map(label => label.name);

The run showing red is attempt 2 of the original run created on 05 August, when the PR had no labels. Re-running replays that original payload, so it will keep failing no matter how many times it is retried, even though patch is on the PR now.

Adding the labels at 23:10 did fire fresh runs, which is the right trigger since the workflow listens on labeled. Those runs are sitting at action_required rather than having run. So the check that would actually pass is waiting on workflow approval, not on anything in the branch.

I cannot clear it myself, adding a label returns 403 Must have admin rights, so this needs you either way.

codecov/project is also red. I will look at that once the gated test runs have actually produced coverage, since right now there is nothing current to compare against.

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

Labels

enhancement New feature or request patch Increment the patch version when merged UX

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unclear error for files with timestamps that are not timezone aware with dandi organize

2 participants