-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Support multiple HTTP inputs #6333
Conversation
cde99a2
to
9d5225a
Compare
Looks like AppVeyor failed on an |
9d5225a
to
e4395f8
Compare
e4395f8
to
aac8f4e
Compare
aac8f4e
to
0d860f0
Compare
LGTM 👍 |
This is looks like a breaking change to the config file. Are there any other inputs that are limited to only one? It would be nice to not have the config break. Can we keep the existing HTTP config option and add it to the slice for backwards compatibility? |
@jwilder I'm planning to open another PR to do exactly that for all the listeners but haven't finished yet. |
@gunnaraasen What happens when you use an old config that still uses |
@@ -9,6 +9,7 @@ | |||
- [#5707](https://github.com/influxdata/influxdb/issues/5707): Return a deprecated message when IF NOT EXISTS is used. | |||
- [#6334](https://github.com/influxdata/influxdb/pull/6334): Allow environment variables to be set per input type. | |||
- [#6394](https://github.com/influxdata/influxdb/pull/6394): Allow time math with integer timestamps. | |||
- [#6333](https://github.com/influxdata/influxdb/pull/6333): Support for multiple listeners for HTTP inputs. |
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.
Needs to be moved to next release.
This needs to be updated to support |
What's the rationale behind this change? Unlike the other configuration changes, I don't see much value to multiple HTTP listeners. There are some downsides though with the naming behind the environment variable overrides and the configuration file change. If someone needs multiple HTTP listeners, what can we do better that Nginx can't do? Otherwise, I think we should just advise people to install a proper web server without risking the breaking change. |
I agree w/ @jsternberg. I don't see the benefit of supporting this. |
Mainly, the benefit would be the ability to set up an unauthenticated localhost listener for administrative access. It would also make this service consistent with the Graphite, OpenTSDB, UDP, and CollectD services. I'm going to close this since it's a large change not necessary for 1.0. |
Required for all non-trivial PRs
Adds support for multiple HTTP listeners. This is the last issue to completely fix #2785.