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

Deprecate es.http.cname_in_publish_address setting #45616

Merged
merged 3 commits into from Aug 22, 2019

Conversation

andrershov
Copy link
Contributor

@andrershov andrershov commented Aug 15, 2019

Follow up on #32806.

The system property es.http.cname_in_publish_address is deprecated starting from 7.0.0 and deprecation warning should be added if the property is specified.
This PR will go to 7.x and master.
Follow-up PR to remove es.http.cname_in_publish_address property completely will go to the master.

@andrershov andrershov added the :Distributed/Network Http and internode communication implementations label Aug 15, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

Copy link
Member

@original-brownbear original-brownbear left a comment

Choose a reason for hiding this comment

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

@andrershov Thanks for finding this!

I think we can't just remove this in the first PR? See the plan in #32806 (review) we need to first adjust the deprecation logging to say that this property does not have an effect anymore (and actually make it so like you did here). Then in another PR to master only we can remove that deprecation logging as well.

-> just keep the deprecation logger + adjust the message + then do a follow up ?

@andrershov
Copy link
Contributor Author

@original-brownbear correct, we need to log deprecation warning if the property is specified in 7.x.
I've updated the code and PR description and it's ready for the review.

@andrershov
Copy link
Contributor Author

run elasticsearch-ci/1

Copy link
Member

@original-brownbear original-brownbear left a comment

Choose a reason for hiding this comment

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

LGMT after fixing the formatting change.

new TransportAddress(localhost, port)
), 0L, false
), NetworkAddress.format(localhost) + ':' + port
new HttpInfo(
Copy link
Member

Choose a reason for hiding this comment

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

Revert noisy formatting change

@andrershov andrershov merged commit a66170a into elastic:master Aug 22, 2019
@andrershov andrershov changed the title Remove deprecated es.http.cname_in_publish_address setting Deprecate es.http.cname_in_publish_address setting Aug 22, 2019
andrershov pushed a commit that referenced this pull request Aug 22, 2019
Follow up on #32806.

The system property es.http.cname_in_publish_address is deprecated
starting from 7.0.0 and deprecation warning should be added if the
property is specified.
This PR will go to 7.x and master.
Follow-up PR to remove es.http.cname_in_publish_address property
completely will go to the master.

(cherry picked from commit a5ceca7)
jkakavas pushed a commit to jkakavas/elasticsearch that referenced this pull request Aug 22, 2019
…5616)

Follow up on elastic#32806.

The system property es.http.cname_in_publish_address is deprecated
starting from 7.0.0 and deprecation warning should be added if the
property is specified.
This PR will go to 7.x and master.
Follow-up PR to remove es.http.cname_in_publish_address property
completely will go to the master.
andrershov pushed a commit that referenced this pull request Aug 22, 2019
Follow-up of #45616.

Starting with 8.0.0 es.http.cname_in_publish_address setting support is
completely removed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed/Network Http and internode communication implementations >non-issue v7.4.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants