config: not properly loaded error #140
Conversation
When a wrong config file was used, it result in a panic. Adding some check condition to validate the Unmarshaled configuration before using it fixes quay#134
| @@ -111,7 +112,10 @@ func Load(path string) (config *Config, err error) { | |||
| return | |||
| } | |||
| config = &cfgFile.Clair | |||
|
|
|||
| if config.API == nil || config.Database == nil || config.Notifier == nil || config.Updater == nil { | |||
There was a problem hiding this comment.
This isn't going to work because the default configuration is loaded first and these fields will never be nil. Instead, we might simply add a check in the database code that tries to open the database.
There was a problem hiding this comment.
Or find a way to verify that there's a clair key somehow.
|
it's what I'm thinking of but that mean unmarshaling yaml than merge I can check for the db connection is the only critical value in the config? You had a health check previously for the db connection isn't it? |
|
From the configuration Load method, it could be overhead to check for the database connection. |
jzelinskie
added
area/usability
|
Could I supposed that only the database source shouldn't be empty? |
| //ErrConfigNotLoaded is returned when the configuration file is not loaded properly | ||
| ErrConfigNotLoaded = errors.New("could not load configuration properly") | ||
| // ErrDatasourceNotLoaded is returned when the datasource variable in the configuration file is not loaded properly | ||
| ErrDatasourceNotLoaded = errors.New("could not load datasource configuration properly") |
There was a problem hiding this comment.
We try to declare the errors are locally as possible - it should be in config/config.go as this error is very specific and not meant to be re-used.
Also, I am not sure that everybody will understand that error message. Maybe something like could not load configuration: no database source specified would be more appropriate?
There was a problem hiding this comment.
OK don't know for the locality of errors. I will move it.
Better message indeed. Thanks for the feedback
|
LGTM. Thank you so much for your contribution! |
When a wrong config file was used, it result in a panic.
Adding some check condition to validate the Unmarshaled configuration
before using it
fixes #134