Open lebogg opened 7 months ago
Hi @lebogg thanks for your suggestion and for testing the aggregator!
If we can improve the messaging here, that looks like a good thing, we'd accept a pull request for this.
Note that in many cases it is fine to have no config file and just use the defaults. However for the aggregator that maybe different from the general case.
What about logging the tried config file path in general. And possible add a hint to the aggregator error message to also check the place of the configuration file.
What happened?
I tried to test the
csaf_aggregator
by building and just running it without any parameter, in particular without a path to a config file. Then, of course, the aggregator was looking into the default paths but couldn't find any config file. I got the following error message:error: no providers given in configuration
This is due a check in theprepare
function ofconfig.go
where the providers of the passed config is checked: https://github.com/csaf-poc/csaf_distribution/blob/086c4ab48b292b05b2382bf7b26d99ed8b346a0d/cmd/csaf_aggregator/config.go#L456-L458What would I expect?
For me, the error code is misleading since it suggests that just the providers are missing in the config file, but the whole config file is missing in the first place. Therefore, having, e.g., a typo in the passed path to the config file lets the user in the unknown that the path or the file was actually wrong. I would expect an error or additional warning message indicating sth. like
Suggestion
A simple solution would be to print a warning log message when the config file couldn't be found in the generic
Parse()
function if it doesn't clash with the usage in other places (it is still just a warning message though). https://github.com/csaf-poc/csaf_distribution/blob/7a8cdb6d19271ac1b9d5a0a93891056a9f3994cc/internal/options/options.go#L47For example: