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

[index_pattern_management]: Replace calls to /elasticsearch/_msearch with internal route. #77564

Merged
merged 4 commits into from
Sep 16, 2020

Conversation

lukeelmers
Copy link
Member

Closes #73993
Related to #55140

Summary

This removes the final usage of the /elasticsearch/_msearch endpoint in Kibana. It was being used by the Index Pattern Management UI when generating previews of scripted field results in the field editor.

Changes include the following:

  • Introduces a preview_scripted_field route that's internal to the index_pattern_management plugin
    • It uses _search instead of _msearch, as I couldn't find any reason why _msearch was actually necessary here (though I tested it with our recently added internal _msearch route and it still worked)
  • Created a routes directory in index_pattern_management and consolidates existing resolve_index route there as well.
  • On the client, requests now go to this new route. Some types had to be slightly adjusted along the way.

Testing

No functional changes introduced here. To test, create a new scripted field and click the "preview" link from the editor. If the script is invalid, an error should be shown in the preview flyout, otherwise a preview should display as it did before.

@lukeelmers lukeelmers added blocker Feature:Data Views Data Views code and UI - index patterns before 8.0 v8.0.0 Team:AppArch release_note:skip Skip the PR/issue when compiling release notes v7.10.0 NeededFor:Core labels Sep 15, 2020
@lukeelmers lukeelmers self-assigned this Sep 15, 2020
script_fields: {
[name]: {
script: {
lang: 'painless',
Copy link
Member Author

Choose a reason for hiding this comment

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

I hardcoded painless here because we removed support for expression based scripted fields awhile ago, thus eliminating the ability to edit existing scripted fields. As a result there was really no point in passing the lang around, since you already can't preview a legacy expression scripted field anyway.

Comment on lines +41 to +48
.then((res) => ({
status: res.statusCode,
hits: res.body.hits,
}))
.catch((err) => ({
status: err.statusCode,
error: err.body.attributes.error,
}));
Copy link
Member Author

Choose a reason for hiding this comment

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

Restructured the response to minimize the amount of changes I needed to make to the existing types that the UI relies on.

const { index, name, script, query, additionalFields } = request.body;

try {
const response = await client.search({
Copy link
Member Author

Choose a reason for hiding this comment

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

Logic here was mostly ripped off from the existing code on the client, with some slight modifications.

@lukeelmers lukeelmers marked this pull request as ready for review September 15, 2020 23:49
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-arch (Team:AppArch)

Copy link
Contributor

@pgayvallet pgayvallet left a comment

Choose a reason for hiding this comment

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

Thanks a lot for doing this, it will unblock #71927 and allow to finally get rid of the legacy es plugin!

Copy link
Contributor

@mattkime mattkime left a comment

Choose a reason for hiding this comment

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

code looks good and works well.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

async chunks size

id value diff baseline
indexPatternManagement 807.2KB -160.0B 807.4KB

distributable file count

id value diff baseline
default 45911 +3 45908
oss 27731 +3 27728
total +6

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

lukeelmers added a commit to lukeelmers/kibana that referenced this pull request Sep 16, 2020
lukeelmers added a commit that referenced this pull request Sep 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocker Feature:Data Views Data Views code and UI - index patterns before 8.0 NeededFor:Core release_note:skip Skip the PR/issue when compiling release notes v7.10.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace usage of /elasticsearch/_msearch endpoint in Index Pattern Management UI
5 participants