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

Unable to override some config options from command line #574

Closed
kozlovic opened this issue Sep 5, 2017 · 0 comments
Closed

Unable to override some config options from command line #574

kozlovic opened this issue Sep 5, 2017 · 0 comments
Assignees

Comments

@kozlovic
Copy link
Member

kozlovic commented Sep 5, 2017

Processing of default options and configuration file is such
that it is sometimes impossible to override default options and/or
options from the configuration file with command line ones.

For instance, any command line parameter that would be set
to its default type value (say false for boolean, "" for string, etc..)
would be ignored and not override default or config file values.

@kozlovic kozlovic added the bug label Sep 5, 2017
@kozlovic kozlovic self-assigned this Sep 5, 2017
kozlovic added a commit that referenced this issue Sep 5, 2017
The idea is to call flag.Parse() twice. The first time - as it was
the case before - is setting the empty Options structure with all
the specified flag defaults and with values provided from the command
line.
If a config file is specified, we then apply to this options structure
the content of the config file. This is done with a new receiver
function for the Options structure. After that, we call flag.Parse() again
which will override the latest changes with any command line parameter.
We don't need MergeOptions() anymore since the above process takes
care of the proper override.

Resolves #574
@ghost ghost added in progress labels Sep 5, 2017
kozlovic added a commit that referenced this issue Sep 6, 2017
There were some cases where override would not work. Any command
line parameter that would be set to the type default value (false
for boolean, "" for string, etc) would not be taken into account.

I moved all the flags parsing and options configuration into
a new function, which may help reduce code duplication in
NATS Streaming.

The other advantage of moving this in a function is that it
can now be unit tested.

I am also removing call to `RemoveSelfReference()` which attempted
to remove a route to self, which has been already solved at runtime
with detecting and ignoring a route to self.

This function would be invoked only when routes were defined in
the configuration file, not in the command line parameter.

Removing this call also solves an user issue (#577)

Resolves #574
Resolves #577
@ghost ghost removed the in progress label Sep 7, 2017
@bruth bruth removed the 🐞 bug label Aug 18, 2023
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

No branches or pull requests

2 participants