update client node detection method#5556
Conversation
0074ab5 to
bc50cfa
Compare
|
LGTM |
|
I'm not really familiar with this, so I am not a good person to give this a second review, but I do have a proposal to help us avoid inconsistencies like this in the future: It'd be great if we started moving more and more of this type of functionality into standalone modules that could be imported into multiple projects. Basically any time we're implementing similar logic in two different repos (i.e. kibana and marvel), we could instead be implementing it in a single place and then have those repos simply import it as a dependency. |
There was a problem hiding this comment.
does it make sense to use esBool(attrs.master), esBool(attrs.data) here? I'm not sure if it's ever anything other than 'false' but under the assumption the method was made for a reason
There was a problem hiding this comment.
I would agree with this, and also add I'm not very comfortable with comparison of 'false' as a String.
a43ddee to
82b2d65
Compare
82b2d65 to
ee19328
Compare
|
I was convinced about preserving
@epixa since all the plugins are going to use Kibana as the app infrastructure, how about we integrating this logic into the elasticsearch plugin and exporting it so other plugins could share? The only downside I see to this, is that if we keep going down this path, the cross-sharing of data and methods between plugins could become quite a jungle. |
|
Talked to @rashidkpc and @spalger about this. Seems like it would be better to not filter out client nodes at all. Although most of the time having old versions of client nodes will not break anything, client nodes do some preprocessing of the request, so there are no guarantees that old client nodes won't break things by preprocessing the request in a way that up-to-date versions of elasticsearch can't support. |
|
Also, users that want to avoid upgrading client nodes can use http instead of the binary protocol as a work around |
I was looking at how Kibana determines that it's compatible with the versions of Elasticsearch on all the nodes in the cluster, and I saw this code that detects client nodes. I've seen a newer method for doing this, and figured it should be updated here.
To test:
bin/elasticsearch --node.data=false --node.master=false)