-
-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
Ensure filename is shown when YAML raises an error #6139
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #6139 +/- ##
==========================================
+ Coverage 53.69% 53.70% +0.01%
==========================================
Files 50 50
Lines 9398 9403 +5
Branches 1652 1653 +1
==========================================
+ Hits 5046 5050 +4
- Misses 4053 4055 +2
+ Partials 299 298 -1 ☔ View full report in Codecov by Sentry. |
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.
LGTM, tested here with success! Also reports the correct location in the yaml file for broken lambdas.
I just noticed your change to make the Dashboard use the C loader too: I don't use the Dashboard much myself (and I'm even less familiar with its code), so I'm not sure if that's been affected or not. Worth checking? |
It's all the same code path so if it's solved here it's solved there |
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.
LGTM, too -- thanks! 🍻
What does this implement/fix?
We were doing the file I/O ourselves. Since PyYAML gives a much better error if we let it do the I/O on the file handle (for all loaders), we pass the handle in instead. This mirrors how we load yaml in HA.
fixes esphome/issues#5423
fixes esphome/issues#5377
Types of changes
Related issue or feature (if applicable): fixes
Pull request in esphome-docs with documentation (if applicable): esphome/esphome-docs#
Test Environment
Example entry for
config.yaml
:# Example config.yaml
Checklist:
tests/
folder).If user exposed functionality or configuration variables are added/changed: