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

feat: raise UnsupportedExtensionException when loading .yml files #2890

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ndormann
Copy link

@ndormann ndormann commented Apr 11, 2024

Motivation

When trying to open a config with the unsupported extension .yml a misleading MissingConfigException is returned.
For details see #2889
The proposed change raises a UnsupportedExtensionException with a helpful error message.

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

The test test_load_yml_file has been adapted to check that the exception is raised correctly.

Related Issues and PRs

#2889

@ndormann ndormann force-pushed the feat/exception_on_invalid_file branch from 1af7b56 to 4e87b94 Compare April 11, 2024 01:05
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 11, 2024
@ndormann ndormann force-pushed the feat/exception_on_invalid_file branch from 4e87b94 to 7f3bc1d Compare April 11, 2024 14:01
@ndormann ndormann changed the title feat: raise UnsupportedExtensionError when loading .yml files feat: raise UnsupportedExtensionException when loading .yml files Apr 11, 2024
Copy link
Collaborator

@Jasha10 Jasha10 left a comment

Choose a reason for hiding this comment

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

Thank you @ndormann.

tests/test_config_loader.py Dismissed Show dismissed Hide dismissed
hydra/errors.py Dismissed Show dismissed Hide dismissed
@Jasha10 Jasha10 linked an issue Apr 11, 2024 that may be closed by this pull request
2 tasks
@ndormann
Copy link
Author

@Jasha10 what is the status on merging this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Misleading error message when opening .yml file
3 participants