-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
[confmap] Add error hint when invalid YAML was passed #12180
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #12180 +/- ##
=======================================
Coverage 92.19% 92.19%
=======================================
Files 465 465
Lines 25194 25204 +10
=======================================
+ Hits 23228 23238 +10
Misses 1570 1570
Partials 396 396 ☔ View full report in Codecov by Sentry. |
One thing I noticed a lot while researching this issue is that a lot of the test cases only test that some kind of error was produced, and not the content of the error. Perhaps you could consider fixing that as part of this PR. Examples:
Improving these tests would prevent a regression like this in the future, they're easy to find by searching for |
@mattsains That makes sense. I am not going to work on this further at least until next week, if you want to file a PR adding the |
3ea155d
to
6d4a99e
Compare
@mattsains Can you take another look? |
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 to me! Although I'm not a maintainer on this repo
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
Not stale, needs approver |
cc @open-telemetry/collector-approvers PTAL! |
Description
Add error hint field to surface YAML parsing errors. This only surfaces the error for top-level URIs but it's a step in the right direction.
Error when passing a file with contents
[invalid:,
as the config:Before:
After:
Link to tracking issue
Updates #12000
Testing
Documentation