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

rename config option .url and .ca to .hosts and .certificate_authorit #10380

Merged

Conversation

colinsurprenant
Copy link
Contributor

@colinsurprenant colinsurprenant commented Feb 4, 2019

Per the recent Kibana options changes in kibana#29496 we are also changing this options for 7.0.0-beta1.

  • *.elasticsearch.url to *.elasticsearch.hosts
  • *.elasticsearch.ca to *.elasticsearch.certificate_authority

This PR has 3 commits; one for the code changes, tests and docs.

Copy link
Member

@yaauie yaauie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good.

Do we need to deprecate these settings on the 6.x branch before 6.7 is cut?

@@ -26,8 +26,8 @@ def additionals_settings(settings)
settings.register(LogStash::Setting::ArrayCoercible.new("xpack.management.pipeline.id", String, ["main"]))
settings.register(LogStash::Setting::NullableString.new("xpack.management.elasticsearch.username", "logstash_system"))
settings.register(LogStash::Setting::NullableString.new("xpack.management.elasticsearch.password"))
settings.register(LogStash::Setting::ArrayCoercible.new("xpack.management.elasticsearch.url", String, [ "https://localhost:9200" ] ))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will we need a backport in which we merely deprecate the current behaviour for 6.x (6.7, which is cut tomorrow)

@colinsurprenant
Copy link
Contributor Author

@jsvd WDYT for the deprecation?

@jsvd
Copy link
Member

jsvd commented Feb 5, 2019

@colinsurprenant thinking about how users will take their logstash.yml configuration and apply it to 7.0.0 this will present a breaking change without any heads up or message pointing them to the right setting, so having 6.7 deprecate these settings would be ideal. can you please give it a go?

@colinsurprenant
Copy link
Contributor Author

rebasing and merging.

@colinsurprenant
Copy link
Contributor Author

unrelated failure #10371 - merging.

@colinsurprenant colinsurprenant merged commit 6990d08 into elastic:master Feb 5, 2019
colinsurprenant added a commit that referenced this pull request Feb 5, 2019
ycombinator added a commit to elastic/logstash-docker that referenced this pull request Feb 8, 2019
In keeping with the changes made in elastic/logstash#10380.
ycombinator added a commit to elastic/logstash-docker that referenced this pull request Feb 8, 2019
In keeping with the changes made in elastic/logstash#10380.
ycombinator added a commit to elastic/logstash-docker that referenced this pull request Feb 8, 2019
In keeping with the changes made in elastic/logstash#10380.
ycombinator added a commit to elastic/logstash-docker that referenced this pull request Feb 8, 2019
In keeping with the changes made in elastic/logstash#10380.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants