Skip to content

Index Patterns API - Remove legacy es client usage for field caps#80116

Merged
mattkime merged 22 commits intoelastic:masterfrom
mattkime:field_caps_remove_legacy_es_client_usage
Oct 23, 2020
Merged

Index Patterns API - Remove legacy es client usage for field caps#80116
mattkime merged 22 commits intoelastic:masterfrom
mattkime:field_caps_remove_legacy_es_client_usage

Conversation

@mattkime
Copy link
Copy Markdown
Contributor

@mattkime mattkime commented Oct 12, 2020 •
edited
Loading

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
@lukasolson lukasolson mentioned this pull request Oct 15, 2020
Merged
3 tasks
@justinkambic justinkambic mentioned this pull request Oct 16, 2020
Merged
8 tasks
@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
Copy Markdown
Contributor

elasticmachine commented Oct 20, 2020

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

cjcenizal
cjcenizal approved these changes Oct 20, 2020
View reviewed changes
Copy link
Copy Markdown
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 - DEPRECATED Use Team:obs-ux-infra_services. Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability labels Oct 20, 2020
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Oct 20, 2020

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

@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Oct 20, 2020

Pinging @elastic/uptime (Team:uptime)

lukasolson
lukasolson approved these changes Oct 20, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@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.

mikecote
mikecote reviewed Oct 21, 2020
View reviewed changes
justinkambic
justinkambic approved these changes Oct 21, 2020
View reviewed changes
Copy link
Copy Markdown
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.

kertal
kertal approved these changes Oct 21, 2020
View reviewed changes
Copy link
Copy Markdown
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 👍

jasonrhodes
jasonrhodes reviewed Oct 21, 2020
View reviewed changes
x-pack/plugins/infra/server/lib/adapters/framework/kibana_framework_adapter.ts
rest[1].allowNoIndices = true;
return requestContext.core.elasticsearch.legacy.client.callAsCurrentUser(...rest);
});
return new IndexPatternsFetcher(requestContext.core.elasticsearch.client.asCurrentUser, true);
Copy link
Copy Markdown
Member

@jasonrhodes jasonrhodes Oct 21, 2020

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
Copy Markdown
Contributor Author

@mattkime mattkime Oct 21, 2020

Choose a reason for hiding this comment

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

correct!

Copy link
Copy Markdown
Member

@jasonrhodes jasonrhodes Oct 21, 2020

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??? ;)

XavierM
XavierM approved these changes Oct 22, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@XavierM XavierM left a comment •
edited
Loading

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!

mikecote
mikecote approved these changes Oct 22, 2020
View reviewed changes
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

mattkime commented Oct 23, 2020

@elasticmachine merge upstream

@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Oct 23, 2020

💚 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
Copy Markdown
Contributor

kibanamachine commented Oct 27, 2020

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 mattkime mentioned this pull request Oct 27, 2020
Merged
mattkime added a commit to mattkime/kibana that referenced this pull request Oct 27, 2020
@mattkime
Index Patterns API - Remove legacy es client usage for field caps (el…
b8bd2b7 
…astic#80116)

* remove legacy es client usage
@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
@mattkime
Index Patterns API - Remove legacy es client usage for field caps (#8…
41206e1 
…0116) (#81784)

* remove legacy es client usage
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pmuellr pmuellr pmuellr left review comments

@jasonrhodes jasonrhodes jasonrhodes left review comments

@XavierM XavierM XavierM approved these changes

@kertal kertal kertal approved these changes

@lukasolson lukasolson lukasolson approved these changes

@mikecote mikecote mikecote approved these changes

@justinkambic justinkambic justinkambic approved these changes

+2 more reviewers

@sorenlouv sorenlouv sorenlouv approved these changes

@cjcenizal cjcenizal cjcenizal approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

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 - DEPRECATED Use Team:obs-ux-infra_services. Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v7.11.0 v8.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.