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

Deprecation warnings for scripted fields #9193

Merged
merged 3 commits into from
Dec 12, 2016

Conversation

Bargs
Copy link
Contributor

@Bargs Bargs commented Nov 22, 2016

This PR adds deprecation warnings on the scripted field list page and the scripted field creation page when languages other than expression and painless are being used.

If any existing scripted fields are using a deprecated language, they'll see this warning:

screen shot 2016-11-22 at 5 34 57 pm

If a user attempts to create a new scripted field with a deprecated language they'll see this warning:

screen shot 2016-11-22 at 5 34 20 pm

To test you'll need to enable at least groovy in Elasticsearch. You can do this easily by modifying your esvm.js file and adding the following block to the config:

script: {
  engine: {
    groovy: {
      inline: true
    }
  }
}

Fixes #9174

@Bargs Bargs added the v5.1.0 label Nov 22, 2016
@Bargs Bargs added the review label Nov 22, 2016
@tbragin tbragin added the Feature:Scripted Fields Scripted fields features label Nov 23, 2016
Copy link
Member

@weltenwort weltenwort left a comment

Choose a reason for hiding this comment

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

Good stuff, left just a few suggestions in the comments.

</h4>
<p>
<span class="text-capitalize">{{editor.field.lang}}</span> is deprecated and support will be removed in the
next major version of Kibana and Elasticsearch. We recommend using Painless for new scripted fields.
Copy link
Member

Choose a reason for hiding this comment

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

Linking to the Painless docs in the index-level warning is a great idea. Let's do the same here.

$scope.getDeprecatedLanguagesInUse = function () {
const fields = $scope.indexPattern.getScriptedFields();
const langsInUse = _.uniq(_.map(fields, 'lang'));
return _.difference(langsInUse, ['expression', 'painless']);
Copy link
Member

Choose a reason for hiding this comment

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

Extracting the array of supported languages into a constant somewhere would allow to reduce duplication with the field editor.

@@ -87,6 +87,10 @@ uiModules
});
};

self.isSupportedLang = function (lang) {
return _.contains(['expression', 'painless'], lang);
Copy link
Member

Choose a reason for hiding this comment

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

Extracting the array of supported languages into a constant somewhere would allow to reduce duplication with the scripted field list.

* Added painless doc link to field editor deprecation warning
* Extracted scripting lang logic into a reuseable module
* Fixed issue where deprecation warning showed on field editor page if
  scripting was completely disabled in ES
@Bargs
Copy link
Contributor Author

Bargs commented Nov 29, 2016

@weltenwort I've addressed your comments, ready for another look!

@epixa epixa added v5.2.0 and removed v5.1.0 labels Nov 30, 2016
Copy link
Member

@weltenwort weltenwort left a comment

Choose a reason for hiding this comment

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

LGTM

@Bargs
Copy link
Contributor Author

Bargs commented Dec 7, 2016

Gonna wait to merge until we decide if #9172 will go into 5.x, cause I'd like to test them together if we do.

@Bargs
Copy link
Contributor Author

Bargs commented Dec 12, 2016

Leaving #9172 in 6.0 only for now, so merging in 3.. 2.. 1.. 💥

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants