Skip to content

Fix datalad special remote error reporting#7333

Merged
bpoldrack merged 1 commit into
datalad:maintfrom
bpoldrack:fix-7332
Mar 22, 2023
Merged

Fix datalad special remote error reporting#7333
bpoldrack merged 1 commit into
datalad:maintfrom
bpoldrack:fix-7332

Conversation

@bpoldrack

@bpoldrack bpoldrack commented Mar 16, 2023

Copy link
Copy Markdown
Member

Utilize CapturedException.format_with_cause instead of a custom solution, potentially leading to an error message containing an uninformative [None].
This would also include more than just the first-level __cause__ (all the way down and include potential __context__ as well)

Closes #7332

With this change the output reported in #7332 (Failed to download from any of 2 locations [None]) changes to:

[{'action': 'get',
  'annexkey': 'MD5E-s994556226--ffc1353e08d640c7ba3154d963573732.nii.gz',
  'error_message': 'Failed to download from any of 2 locations ["Failed to '
                   'acquire lock to establish download session at '
                   '/home/<REDACTED>.cache/datalad/locks/downloader-auth.lck '
                   'in 3 attempts.Check following process: PID=2946262 '
                   'CWD=/var/lib/condor/execute/dir_2944629/tmpwoojnx0_/datadir/HCP1200/567052/MNINonLinear '
... <REDACTED>

So, much more telling.

@bpoldrack bpoldrack changed the title To be amended Fix datalad special remote error reporting Mar 16, 2023
@bpoldrack bpoldrack added the semver-patch Increment the patch version when merged label Mar 16, 2023
@bpoldrack
bpoldrack marked this pull request as ready for review March 16, 2023 13:22
Comment thread datalad/customremotes/datalad.py Outdated
f"{ce.format_with_cause()}"
self.message(debug_msg)
error_causes.append(cause)
error_causes.append(ce.tb.__cause__)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see complaints from codecov which means that this code block is probably tests covered -- could you add checking for expected cause to appear somewhere where expected in the test?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see where this is covered. test_datalad.py doesn't appear to test retrieval at all.

Utilize `CapturedException`'s `format_with_cause` instead of a custom
solution, potentially leading to an error message containing an
uninformative `[None]`.

Closes datalad#7332
@codecov

codecov Bot commented Mar 17, 2023

Copy link
Copy Markdown

Codecov Report

Patch coverage has no change and project coverage change: +1.98 🎉

Comparison is base (c6004a6) 88.71% compared to head (8c7bc10) 90.70%.

Additional details and impacted files
@@            Coverage Diff             @@
##            maint    #7333      +/-   ##
==========================================
+ Coverage   88.71%   90.70%   +1.98%     
==========================================
  Files         327      327              
  Lines       44518    44530      +12     
  Branches     5908     5909       +1     
==========================================
+ Hits        39494    40390     +896     
+ Misses       5009     4125     -884     
  Partials       15       15              
Impacted Files Coverage Δ
datalad/customremotes/datalad.py 64.28% <0.00%> (+2.64%) ⬆️

... and 14 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@bpoldrack
bpoldrack merged commit f7bc256 into datalad:maint Mar 22, 2023
@yarikoptic-gitmate

Copy link
Copy Markdown
Collaborator

PR released in 0.18.3

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

Labels

semver-patch Increment the patch version when merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

datalad special remote's reporting on error causes is suboptimal

3 participants