Updating Console to work with ES 5.0 APIs #6925
Conversation
ycombinator
changed the title
ycombinator
changed the title
|
I made these changes by looking at https://www.elastic.co/guide/en/elasticsearch/reference/master/breaking-changes-5.0.html and all the sub-pages linked off that page. |
|
I'm not familiar with the Sense code base. How the heck do we test these sort of changes? |
|
I manually tested them by issuing the bad (no longer supported) requests to Elasticsearch 5.0, made sure Elasticsearch returned an error, then removed the corresponding autocomplete rules from Console. |
| 'properties': { | ||
| '*': { | ||
| type: { | ||
| __one_of: ['string', 'float', 'double', 'byte', 'short', 'integer', 'long', 'date', 'boolean', | ||
| __one_of: ['text', 'keyword', 'string', 'float', 'double', 'byte', 'short', 'integer', 'long', 'date', 'boolean', |
|
General comment: Remove deprecated stuff. Its more important that Console nudges users in the right direction than support everything that's valid for a particular version of ES. |
| } | ||
| }); | ||
| api.addEndpointDescription(endpoint, { | ||
| match: endpoint, |
There was a problem hiding this comment.
These don't also get a methods like below?
There was a problem hiding this comment.
It looks like this is so because methods defaults to ['GET'] as per https://github.com/ycombinator/kibana/blob/gh-6913/src/plugins/console/api_server/api.js#L27-L31. That being said, I agree that this is inconsistent — sometimes methods: ['GET'] is explicitly specified in the call to addEndpointDescription and sometimes it is not.
I think this should be cleaned up and made consistent but I'd like to scope this cleanup to the code under the es_5_0 folder only, as the scope of this PR is to make Console work with ES 5.0 APIs. What do you think?
There was a problem hiding this comment.
I would have to agree with you man! No worries I was mainly asking here to try and assess the differences between this function and the one below it. I would have to definitely agree that this should be deferred to the scope of another PR.
| script: { | ||
| // populated by a global rule | ||
| } | ||
| }, field_metric = { |
There was a problem hiding this comment.
This goes against our style guide with comma separated var instantiation.
There was a problem hiding this comment.
Thanks @panda01. I think the entire Console codebase needs to be brought up to speed with the Kibana style guide, now that Console is part of Kibana core. I would prefer to do that outside this PR to keep the changes here scoped to making Console work with Elasticsearch 5.0 APIs. What do you think?
There was a problem hiding this comment.
As i mentioned above, this is cool to be pushed to something else, this is a big PR already.
| var version = es.getVersion() || []; | ||
| var api; | ||
|
|
||
| switch (version[0]) { |
|
LGTM |
Resolves #6913