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
Added option for configuration file #28
Conversation
mjavier2k
commented
Jul 2, 2020
- added option to use config.yaml
- updated README.md
implements #25 |
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.
How do the settings from viper get bound to the config variables?
When viper.ReadInConfig() is called, it reads the config file which has the same key as whats on https://github.com/mjavier2k/solidfire-exporter/blob/master/pkg/solidfire/types.go#L8-L14 that becomes the default values for those KV pair which can be override by either flags or env. |
What I meant is that we're not doing any 'reading' of the viper config anywhere after it exists, i.e. |
My Bad - I forgot that we are already using viper, so your config file pattern just magically works solidfire-exporter/pkg/solidfire/solidfire.go Lines 34 to 36 in ca5e825
|
Do we want to drop support for passing in username/password as flags? |
Yeah, lets drop that support. i'll remove the username/password option on the command line flag |
Is there a need for a config flag for the config file itself?
|
cmd/solidfire-exporter/main.go
Outdated
@@ -36,12 +36,26 @@ func init() { | |||
} else { | |||
viper.BindEnv(solidfire.ListenPortFlag, solidfire.ListenPortFlagEnv) | |||
} | |||
|
|||
viper.BindEnv(solidfire.UsernameFlag, solidfire.UsernameFlagEnv) |
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.
If we remove these here, can we still pass username/password through an environment variable?
As I recall, we want to support username and password via an Environment variable or config file, but want to drop support for having them as command-line flags.
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.
Yup. thats what we want here. going to update this.
@@ -12,9 +12,10 @@ var ( | |||
EndpointFlag = "endpoint" | |||
InsecureSSLFlag = "insecure" | |||
HTTPClientTimeoutFlag = "timeout" | |||
ListenPortFlagEnv = "SOLIDFIRE_PORT" | |||
ConfigFileFlag = "config.yaml" |
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.
This is the flag name not the flag's default value.
You probably meant this to be as such:
ConfigFileFlag = "config.yaml" | |
ConfigFileFlag = "config" |