Skip to content

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.

config_search_path=create_config_search_path(path)
)
with raises(UnsupportedExtensionException):
cfg = config_loader.load_configuration(

Check warning

Code scanning / CodeQL

Variable defined multiple times

This assignment to 'cfg' is unnecessary as it is [redefined](1) before this value is used.
class HydraDeprecationError(HydraException): ...


class UnsupportedExtensionException(HydraException): ...

Check notice

Code scanning / CodeQL

Statement has no effect

This statement has no effect.
@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