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

Change single udp config to support multiple UDP listeners #3599

Merged
merged 1 commit into from
Aug 10, 2015
Merged

Change single udp config to support multiple UDP listeners #3599

merged 1 commit into from
Aug 10, 2015

Conversation

tpitale
Copy link
Contributor

@tpitale tpitale commented Aug 9, 2015

Resolves #3596

In 0.7.1 multiple udp listeners could be configured. I upgraded to 0.9.2 and this feature appeared to be missing. This, hopefully, adds it back.

It does replace the single TOML configuration for [udp] with [[udp]]. I'm not sure if/how this should be handled. This may be considered a breaking change, but since the ability to use multiple servers was already lost (a breaking change) … this seems reasonable to add back in.

…listeners

do not use NewConfig for UDP

appendUDPService must return a value

udp service does not need to handle error

fix missing case of c.UDP in tests
@tpitale
Copy link
Contributor Author

tpitale commented Aug 9, 2015

Would it be preferable to retain [udp], and make the array TOML [[udps]] or something?

@otoolep
Copy link
Contributor

otoolep commented Aug 10, 2015

This seems to me to be a worthwhile change -- thanks @tpitale. Have you signed the CLA?

This is a breaking change, as you say, so we'll need to decide how best to handle it.

@tpitale tpitale changed the title Change (previously unused) single udp config to support multiple UDP listeners Change single udp config to support multiple UDP listeners Aug 10, 2015
@tpitale
Copy link
Contributor Author

tpitale commented Aug 10, 2015

@otoolep I signed the CLA for my last change #591

@tpitale
Copy link
Contributor Author

tpitale commented Aug 10, 2015

Partially resolves #2785 and #2278.

@otoolep
Copy link
Contributor

otoolep commented Aug 10, 2015

+1.

@otoolep
Copy link
Contributor

otoolep commented Aug 10, 2015

Confirmed CLA has been signed.

@jwilder
Copy link
Contributor

jwilder commented Aug 10, 2015

👍

otoolep added a commit that referenced this pull request Aug 10, 2015
Change single udp config to support multiple UDP listeners
@otoolep otoolep merged commit 7f35d65 into influxdata:master Aug 10, 2015
@otoolep
Copy link
Contributor

otoolep commented Aug 10, 2015

Thanks @tpitale

otoolep added a commit that referenced this pull request Aug 10, 2015
This reverts commit 7f35d65, reversing
changes made to f828781.
otoolep added a commit that referenced this pull request Aug 10, 2015
@tpitale tpitale deleted the multiple-udp-servers branch August 17, 2015 20:15
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.

3 participants