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 Patterns API - Remove legacy es client usage for field caps #80116

Merged

Conversation

mattkime
Copy link
Contributor

@mattkime mattkime commented Oct 12, 2020

Summary

Changing Index Pattern's IndexPatternFetcher field fetcher class to use the new es client instead of the legacy client.

@elastic elastic deleted a comment from kibanamachine Oct 12, 2020
@elastic elastic deleted a comment from kibanamachine Oct 12, 2020
@mattkime mattkime changed the title Field caps remove legacy es client usage Index Patterns API - Remove legacy es client usage for field caps Oct 13, 2020
@mattkime mattkime added Feature:Data Views Data Views code and UI - index patterns before 8.0 Team:AppArch v8.0.0 v7.11.0 release_note:skip Skip the PR/issue when compiling release notes labels Oct 13, 2020
@mattkime mattkime requested a review from a team October 20, 2020 21:04
@mattkime mattkime requested review from a team as code owners October 20, 2020 21:04
@elasticmachine
Copy link
Contributor

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

Copy link
Contributor

@cjcenizal cjcenizal left a comment

Choose a reason for hiding this comment

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

I'm rubberstamping this to dismiss the ES-UI codeowner review request triggered by the change to the rollup plugin, butI haven't actually reviewed anything.

@botelastic botelastic bot added Team:APM All issues that need APM UI Team support Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability labels Oct 20, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/apm-ui (Team:apm)

@elasticmachine
Copy link
Contributor

Pinging @elastic/uptime (Team:uptime)

Copy link
Member

@lukasolson lukasolson left a comment

Choose a reason for hiding this comment

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

LGTM! Tested creating index patterns/refreshing mappings, timelion suggestions. Things seem to be working fine. Looks like some docs need to be updated to pass CI.

Copy link
Contributor

@justinkambic justinkambic left a comment

Choose a reason for hiding this comment

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

Uptime changes LGTM! Tested locally with an alert and all seemed to work.

Copy link
Member

@kertal kertal left a comment

Choose a reason for hiding this comment

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

From KibanaApp team side, there was just the TSVB code fetching fields affected, LGTM, tested locally, works. Also tested to refresh the fields of an index pattern, works. LGTM 👍

rest[1].allowNoIndices = true;
return requestContext.core.elasticsearch.legacy.client.callAsCurrentUser(...rest);
});
return new IndexPatternsFetcher(requestContext.core.elasticsearch.client.asCurrentUser, true);
Copy link
Member

Choose a reason for hiding this comment

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

What is true signifying in this signature? (I assume it's the allowNoIndices flag based on what was there before but just checking?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

correct!

Copy link
Member

Choose a reason for hiding this comment

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

Thanks! I think my least favorite thing about TS is that it let us all go back to these dangling signatures :P When does GitHub get full TS support??? ;)

Copy link
Contributor

@XavierM XavierM left a comment

Choose a reason for hiding this comment

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

Tested locally => LGTM on the security solution!

Copy link
Contributor

@mikecote mikecote left a comment

Choose a reason for hiding this comment

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

LGTM from my previous comments

@mattkime
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

@mattkime mattkime merged commit 50a9322 into elastic:master Oct 23, 2020
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 80116 or prevent reminders by adding the backport:skip label.

@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Oct 27, 2020
mattkime added a commit to mattkime/kibana that referenced this pull request Oct 27, 2020
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Oct 27, 2020
mattkime added a commit that referenced this pull request Oct 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Data Views Data Views code and UI - index patterns before 8.0 release_note:skip Skip the PR/issue when compiling release notes Team:APM All issues that need APM UI Team support Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v7.11.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.