Say why metadata failed to load and how to see the tracebacks - #1901
Say why metadata failed to load and how to see the tracebacks#1901adityasingh2400 wants to merge 1 commit into
Conversation
`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
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 ? |
|
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 Before After Three things the old one costs you, visible in that comparison:
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 |
Codecov Report❌ Patch coverage is
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
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Unrelated to the demo, but worth flagging so you do not spend time on it: the red 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 Adding the labels at 23:10 did fire fresh runs, which is the right trigger since the workflow listens on I cannot clear it myself, adding a label returns
|
Fixes #1640
dandi organizewarnedFailed 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 leveland 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 = 5so a large batch does not flood the console. It also says that full tracebacks are written to the log file, and thatdandi --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
masterthe 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 revertedorganize.pyto 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.pyexcluding 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_generatedas CLAUDE.md asks. I reviewed and tested everything before submitting.