-
Couldn't load subscription status.
- Fork 779
Better lookup failure message in Settings.from_name
#1064
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR improves error messaging for configuration file lookup failures in Settings.from_name by showing both lookup locations and preventing the clobbering of user config paths when name collisions occur.
- Reorganizes the config file lookup logic to calculate both user and package paths upfront
- Enhances the
FileNotFoundErrormessage to include both the user config path and bundled config path - Fixes the import statement to use the correct module reference
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
80792ce to
839e2da
Compare
839e2da to
e23583a
Compare
| raise FileNotFoundError( | ||
| f"No configuration file found for {config_name}" | ||
| ) from e | ||
| else: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously without this else we would overwrite the user_config_path location -- @mskarlin does this seem like a bug to you?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice bugfix
This PR helps with #1044
Now the
FileNotFoundErrorlooks like: