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

Strip whitespace from value for ES servers and kafka brokers #1302

Closed
black-adder opened this issue Jan 28, 2019 · 4 comments
Closed

Strip whitespace from value for ES servers and kafka brokers #1302

black-adder opened this issue Jan 28, 2019 · 4 comments
Labels
enhancement good first issue Good for beginners help wanted Features that maintainers are willing to accept but do not have cycles to implement

Comments

@black-adder
Copy link
Contributor

black-adder commented Jan 28, 2019

Requirement - what kind of business use case are you trying to solve?

See #1300 for ES servers and kafka brokers

Problem - what in Jaeger blocks you from solving the requirement?

cfg.servers = v.GetString(cfg.namespace + suffixServerURLs)
and
Brokers: strings.Split(v.GetString(configPrefix+suffixBrokers), ","),

Proposal - what do you suggest to solve the problem or improve the existing situation?

Same solution as in #1301

Any open questions to address

Perhaps come up with a general solution for this? Maybe x/config/viper.go that wraps viper but with an extra function GetStrings that handles the removal of whitespaces? Overkill?

@black-adder black-adder added enhancement help wanted Features that maintainers are willing to accept but do not have cycles to implement good first issue Good for beginners labels Jan 28, 2019
@jpkrohling
Copy link
Contributor

Perhaps come up with a general solution for this?

You mean, to be applied automatically to all properties, or as a helper function? There might be cases where spaces should be part of the value, but we could apply this to properties where the input is well-defined, like IP addresses/hostnames.

@black-adder
Copy link
Contributor Author

a helper function so as to not to copy stripWhitespace function everywhere.

@annanay25
Copy link
Member

@black-adder was this issue fixed with #1305?

@pavolloffay
Copy link
Member

Yeah I think it was, please reopen if it still has to be solved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement good first issue Good for beginners help wanted Features that maintainers are willing to accept but do not have cycles to implement
Projects
None yet
Development

No branches or pull requests

4 participants