-
Notifications
You must be signed in to change notification settings - Fork 33
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
add config_file exemple and docs #93
base: main
Are you sure you want to change the base?
Conversation
README.md
Outdated
@@ -66,7 +75,7 @@ Usage of ./endlessh-go | |||
-logbuflevel int | |||
Buffer log messages logged at this level or lower (-1 means don't buffer; 0 means buffer INFO only; ...). Has limited applicability on non-prod platforms. | |||
-logtostderr | |||
log to standard error instead of files | |||
Log to standard error instead of files |
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.
You should not worry about these.
They are from ./endlessh-go --help
Even if you are changing it now, next time I copy the output of ./endlessh-go --help
will overwrite them again.
If you do want to change text, you need to change at where the flag declared.
|
||
You will find an exemple of a configuration file in the [exemple section](exemple/config_file/endlessh-go.yml) | ||
|
||
**Please note that if both a configuration file and CLI arguments define the same options, the CLI arguments will take precedence and overwrite the configuration file.** |
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.
May I ask why do you select this precedence?
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.
Config file can be shared easily. There you can define most of your configuration, it's like your personnal default config. If you need a small change (for testing purpose for exemple) you can add cli args. That is what i was thinking while writing this.
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.
I think precedence should be :
Command line.
Config file (declared in the command line)
Environment vars
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.
I would say Environment vars should be the highest. That allows you put the same setup into different envs.
I am not sure whether we can customize log via config file at this moment, especially in case of CLI > config file.
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.
The question is not so simple. Will environment variables be used in contexts other than containers?
Should the consideration of environment variables depend on a CLI argument "--use-env-vars"?
To my point of view, Env vars should neither overwrite CLI args neither config file declared with CLI args.
examples/config_file/endlessh-go.yml
Outdated
@@ -0,0 +1,42 @@ | |||
# Connection type. Possible values are tcp, tcp4, tcp6 | |||
conn_type: "tcp" |
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.
yml is fine.
I also want to hear opinion about .env
style file, just like you are declaring environment variables.
ENABLE_PROMETHEUS=false
What do you think?
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.
I made an examples/env.exemple file if you want to tell me what you think about.
examples/config_file/endlessh-go.yml
Outdated
# When logging hits line file:N, emit a stack trace | ||
log_backtrace_at: [] | ||
# If non-empty, write log files in this directory | ||
log_dir: "" |
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 important are these log related flag to you?
They are from libraries I used, so I don't have control over them. The library may add/remove flags.
I may or may not be able to change the behavior. It needs some study.
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.
the log_backtrace_at var is only for debugging purpose and i don't think it is appropriate for release.
log_dir could be usefull for binary use (can't find a use case where it's usefull for containers)
asked in #90 discussion