Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

require_dataset() uniformly raises NoDatasetFound #6521

Merged
merged 2 commits into from
Mar 9, 2022

Conversation

mih
Copy link
Member

@mih mih commented Mar 7, 2022

Before it claimed to raise InsufficientArgumentsError uniformly,
but actually raised NoDatasetFound or ValueError, depending
on the exact error condition.

This change consolidates and documents raising NoDatasetFound,
which seems to make most sense.

Changelog

🐛 Bug Fixes

  • require_dataset() now uniformly raises NoDatasetFound when no dataset was found. Implementations that catch the previously documented InsufficientArgumentsError or the actually raised ValueError will continue to work, because NoDatasetFound is derived from both types. Fixes datalad-configuration dump not working without dataset #6503

mih added 2 commits March 7, 2022 11:37
Before it claimed to raise InsufficientArgumentsError uniformly,
but actually raised NoDatasetFound or ValueError, depending
on the exact error condition.

This change consolidates and documents raising NoDatasetFound,
which seems to make most sense.

Coincidentally, this also fixes datalad#6503
@codeclimate
Copy link

codeclimate bot commented Mar 7, 2022

Code Climate has analyzed commit feb4ac3 and detected 1 issue on this pull request.

Here's the issue category breakdown:

Category Count
Security 1

View more on Code Climate.

@mih
Copy link
Member Author

mih commented Mar 7, 2022

Metalad's meta_extract() is specifically catching a NoDatasetFound exception and yield error results, but two of its tests do not expect this in all cases. If filed datalad/datalad-metalad#218 which is describing the conditions surrounding this behavior.

@yarikoptic yarikoptic merged commit 9a92da5 into datalad:master Mar 9, 2022
@mih mih deleted the bf-6503 branch March 25, 2022 13:04
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-configuration dump not working without dataset
3 participants