-
Notifications
You must be signed in to change notification settings - Fork 5.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
fix: Clear error message when provided config is not a text file #11787
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.
when providing a URL the request header is updated with req.Header.Add("Accept", "application/toml"), I thought this might suffice and the extra check if it is a text file shouldn't be necessary and its the servers responsibility.
I agree with this, they will still receive the message that it isn't valid TOML.
Weird, the tests in Windows are failing in the CI because of this change but I can't reproduce it locally on a Windows machine (https://app.circleci.com/pipelines/github/influxdata/telegraf/12585/workflows/9895e703-3964-4ddf-ae2a-91aef8ecaab1/jobs/203622). I thought maybe it had to do with the encoding because Updated the logic to use the mimetype returned by |
Download PR build artifacts for linux_amd64.tar.gz, darwin_amd64.tar.gz, and windows_amd64.zip. 👍 This pull request doesn't change the Telegraf binary size 📦 Click here to get additional PR build artifactsArtifact URLs |
) (cherry picked from commit b5f7ca4)
resolve: #8753
The issue was providing a non text file to Telegraf as a config would cause the error
Error parsing data: line 1: invalid TOML syntax
which isn't that helpful. I couldn't find a good way to detect the type of a file without reading it, you could detect it by extension: https://pkg.go.dev/mime?utm_source=godoc#TypeByExtension but testing this on windows I wouldn't get a result for.toml
. So I chose to use: https://pkg.go.dev/golang.org/x/tools/godoc/util?utm_source=godoc#IsText which will look at the contents of the file to determine if it is a text file.This does mean that depending on the binary size it could be slow to print this error message, but hopefully the error message will help regardless. Because when providing a URL the request header is updated with
req.Header.Add("Accept", "application/toml")
, I thought this might suffice and the extra check if it is a text file shouldn't be necessary and its the servers responsibility. But open to consider this differently.