Skip to content
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

cmd: Add fail-fast approach in case of incorrect config file #2198

Merged
merged 3 commits into from
Jan 24, 2022

Conversation

leszko
Copy link
Contributor

@leszko leszko commented Jan 20, 2022

What does this pull request do? Explain your changes. (required)
Fail fast when there is an error in the Livepeer config file.

Specific updates (required)
N/A

How did you test each of these updates (required)

  1. Non-existing config file
$ ./livepeer -config nonExistingFile.conf
F0120 11:58:24.943412   42834 livepeer.go:171] Error using config file: open nonExistingFile.conf: no such file or directory
  1. Config file with a flag that was not defined
$ cat livepeer.conf 
reward false malformed
$ ./livepeer -config livepeer.conf 
F0120 11:59:23.205466   42904 livepeer.go:171] Error using config file: error setting flag "reward" from config file: parse error
  1. Malformed config file
$ cat livepeer.conf 
nonExistingFlag 123
reward false
$ ./livepeer -config livepeer.conf 
F0120 11:59:52.497457   42961 livepeer.go:171] Error using config file: config file flag "nonExistingFlag" not defined in flag set

Does this pull request close any open issues?
fix #2193

Checklist:

@leszko leszko requested a review from yondonfu January 20, 2022 11:01
Copy link
Member

@yondonfu yondonfu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Just a minor question - feel free to address before merging if you agree.

ff.WithConfigFileFlag("config"),
ff.WithEnvVarPrefix("LP"),
ff.WithConfigFileParser(ff.PlainParser),
)
if err != nil {
glog.Fatal("Error using config file: ", err)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC this error message will be not be accurate if CLI flags are used instead of a config file since ff.Parse() does not necessarily parse a config file if one is not provided and CLI flags are passed instead. Perhaps a more descriptive error message would be just: "Error parsing config"? WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, updated in 3c0e0d3

@leszko leszko merged commit 3f05390 into livepeer:master Jan 24, 2022
@leszko leszko deleted the 2193-livepeer-conf-ignored-parameters branch January 24, 2022 11:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Livepeer configuration parameters are ignored
2 participants