-
Notifications
You must be signed in to change notification settings - Fork 523
Add handling for config directory with .yml
/.yaml
extension
#1293
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
base: develop
Are you sure you want to change the base?
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.
Looks good! Nice catch 👍
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 fixes an issue where a directory with a .yml
or .yaml
extension would be incorrectly treated as a configuration file, causing a misleading PermissionError when attempting to open it. The fix adds an explicit file check to ensure only actual files with these extensions are processed as YAML config files, while directories fall through to the existing directory handling logic.
- Added
os.path.isfile()
check to prevent directories with YAML extensions from being treated as config files - Maintains existing functionality for actual YAML config files
- Improves error handling and user experience when config directories are mistakenly created with YAML extensions
@winstonallo thanks! would you please run the pre-commits per the contributing guideline. Also this PR misses a regression test covering the directory with yml extension scenario so a test case like: def test_yaml_directory_handling():
# like create a directory named whatever_config.yml
# verify it's handled as a directory, not a file this test should fail on the develop branch prior to this PR and go green afterwards. |
Hey, sorry for the late response, I will get to that on Wednesday:) |
Hey @Pouyanpi, I added the test and ran the pre-commit hooks:) Let me know if there is anything else I can do |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1293 +/- ##
========================================
Coverage 70.20% 70.20%
========================================
Files 161 161
Lines 16123 16123
========================================
Hits 11319 11319
Misses 4804 4804
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Description
Prevents a (probably mistakenly created) config.yml/ directory from being attempted to open as a regular file, resulting in a misleading PermissionError (source: just happened to me).
Before: PermissionError: [Errno 13] Permission denied: 'config.yml'
After: config.yml/ directory is treated as a valid config directory (falls through to existing directory handling logic).