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

feature: support command line arguments in a configuration file #1823

Merged
merged 8 commits into from Feb 21, 2021
Merged

feature: support command line arguments in a configuration file #1823

merged 8 commits into from Feb 21, 2021

Conversation

bingoohuang
Copy link
Contributor

@bingoohuang bingoohuang commented Feb 20, 2021

direct command line is too long like:

weed -logdir=../mlog master -mdir=../mdir -peers=192.168.126.5:3333,192.168.126.16:3333,192.168.126.18:3333 -port=3333 -defaultReplication=001

after applying this feature, we can use a configuration file like:

# master.conf
logdir=../mlog
mdir=../mdir
peers=192.168.126.5:3333,192.168.126.16:3333,192.168.126.18:3333
port=3333
defaultReplication=001

then refer it like weed master -config=master.conf

Order of precedence of this flag, it is borrowed from https://github.com/namsral/flag with some customized for weed:

  1. Command line options
  2. Environment variables
  3. Configuration file
  4. Default values

So, if someone like command lines, old style is still there. If someone get boring with so many command line arguments, he can use a configuration file as an alternative. Why not!

@chrislusf
Copy link
Collaborator

How to see the full list of command line options in effect?

@chrislusf
Copy link
Collaborator

This breaks the help message for weed -h. It used to have

Use "weed help [command]" for more information about a command.

For Logging, use "weed [logging_options] [command]". The logging options are:
  -alsologtostderr
    	log to standard error as well as files (default true)
  -log_backtrace_at value
    	when logging hits line file:N, emit a stack trace
  -logdir string
    	If non-empty, write log files in this directory
  -logtostderr
    	log to standard error instead of files
  -stderrthreshold value
    	logs at or above this threshold go to stderr
  -v value
    	log level for V logs
  -vmodule value
    	comma-separated list of pattern=N settings for file-filtered logging

After this PR, the corresponding part became:

Use "weed help [command]" for more information about a command.

For Logging, use "weed [logging_options] [command]". The logging options are:
  -config string
    	config file
  -logdir string
    	If non-empty, write log files in this directory

@bingoohuang
Copy link
Contributor Author

It is fixed.

@chrislusf
Copy link
Collaborator

Thanks a lot!

@chrislusf chrislusf merged commit 258e93b into seaweedfs:master Feb 21, 2021
@chrislusf
Copy link
Collaborator

Merged. It would be better if the config file content is printed out when the program starts.

@bingoohuang
Copy link
Contributor Author

Yes, some additional work can be do :

  1. sample config files creation.
  2. config get effected by command-line, env, or config file printing.
  3. final config adopted overview printing.

And any suggestion more ?

@chrislusf
Copy link
Collaborator

One bug with this approach is that the command line option can not duplicate with existing environment variables. For example, if weed filer -path=xxx is defined, but not specified when just running weed filer. The env variable $PATH would be used. This caused a big confusion.

It should only read env variable $WEED_CLI_XXXX, e.g., $WEED_CLI_PATH.

@bingoohuang
Copy link
Contributor Author

Yes, it is really a bug, I should set env prefix to a default "WEED_", a pr will be submit ASAP.

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.

None yet

2 participants