Limit scripted fields to painless and expression langs #9172
Conversation
|
I was wondering if it would make sense to perform that filtering on the server side. Another option would be to move that check to a function named somthing like |
|
I had thought about doing the filtering on the server side, but I really intended for that API to be a view into what's enabled in ES, not just a scripted field specific API. Agreed on creating a separate function though. |
|
@weltenwort pushed a couple updates, ready for another look.
|
| @@ -37,7 +39,7 @@ uiModules | |||
|
|
|||
| self.docLinks = docLinks; | |||
| getScriptingLangs().then((langs) => { | |||
| self.scriptingLangs = langs; | |||
| self.scriptingLangs = _.intersection(langs, ['expression', 'painless']); | |||
There was a problem hiding this comment.
Can we DRY this up as well by re-using getSupportedScriptingLangs()? Maybe even factor out a filterSupportedSciptingLangs(languages) to encapsulate the _.intersection that is also used below? Or does that go too far?
There was a problem hiding this comment.
I kind of like the flexibility of the existing functions. If you're ok with it, let's wait and see if we need a filterSupportedScriptingLangs elsewhere before creating it.
|
I was on the fence about langs vs languages, you've convinced me to switch it :) |
|
@epixa during our Discovery meeting @tbragin and I talked about targeting 5.x with this PR. The idea being that we would continue to allow users to edit existing groovy fields in 5.x but we wouldn't allow them to create new ones. Do you think that's safe, or would you still consider it a breaking change? |
|
Do we have deprecation messages on it already? I might be OK with removing the ability to add new groovy scripted fields if we had a couple versions with the deprecation notice first. I don't like the idea of pulling the rug out without warning though. |
We made a mistake in opening up Kibana scripted fields to any and all langs enabled in Elasticsearch. Kibana needs to do some magic to get certain scripted field features to work (elastic#9171) and we can't commit to supporting anything beyond expression and painless scripts. On top of that, the other languages are being removed in ES 6.0 anyway. This commit makes that stance explicit by disallowing anything other than those two choices.
|
@epixa Not yet, there's a PR for that. |
|
Gonna leave this in 6.0 only for now since we haven't displayed deprecation messages to users yet. We could consider backporting to a 5.x release later on after #9193 has been present in a couple releases. |
We made a mistake in opening up Kibana scripted fields to any and all
langs enabled in Elasticsearch. Kibana needs to do some magic to get
certain scripted field features to work
(#9171) and we can't commit to
supporting anything beyond expression and painless scripts. On top of
that, the other languages are being removed in ES 6.0 anyway. This commit
makes that stance explicit by disallowing anything other than those two
choices.
Tomorrow I'll submit a PR to display deprecation messages in 5.x.