Stop assuming Program Files is on C: drive #6317
Conversation
cmd/telegraf/telegraf.go
Outdated
| @@ -348,7 +348,7 @@ func main() { | |||
| DisplayName: "Telegraf Data Collector Service", | |||
| Description: "Collects data using a series of plugins and publishes it to" + | |||
| "another series of plugins.", | |||
| Arguments: []string{"--config", "C:\\Program Files\\Telegraf\\telegraf.conf"}, | |||
| Arguments: []string{"--config", os.Getenv("ProgramFiles") + "\\Telegraf\\telegraf.conf"}, | |||
There was a problem hiding this comment.
What if the environment variable isn't set, I think we should fallback to C:\Program Files.
There was a problem hiding this comment.
This special environment variable is implicitly set from registry values and AFAIK available in all versions of Windows since XP. Can you explain a reasonable scenario where it isn't set?
There was a problem hiding this comment.
Are you saying it is impossible for it not to be set?
There was a problem hiding this comment.
I'm saying that I am not aware of any situation where it could be unset, since for desktop/server versions of Windows XP+ it should always be set.
Since my knowledge about Window CE/Embedded/IoT is very limited, and since I don't know the oldest Windows version Telegraf is supposed to support off the top of my head, I wanted to leave room for you to prove me wrong, considering you brought up the issue.
Unless you are aware of any such exceptions I suggest we do not clutter down the code with defaults that are never used.
There was a problem hiding this comment.
I'd like this to be handled, any data received from outside the process needs to be checked. We could either have a fallback or exit with an error.
…dle missing env var
|
Thanks @mjiderhamn, looks like we got too close to another edit, can you fix the merge conflict? |
# Conflicts: # cmd/telegraf/telegraf.go
Required for all PRs: