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.

Sign up for GitHub

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

Jump to bottom

Data views - saved object client use resolve instead of get #108637

Merged
merged 18 commits into from
Sep 21, 2021
Merged

Data views - saved object client use resolve instead of get #108637

merged 18 commits into from
Sep 21, 2021

Conversation

mattkime
Copy link
Contributor

@mattkime mattkime commented Aug 15, 2021 •
edited
Loading

Summary

Closes #108335

Data views service should use resolve instead of get endpoint and pass on correct error if there's a conflict.

@mattkime mattkime changed the title so client - use resolve instead of get Index patterns - saved object client use resolve instead of get Aug 15, 2021
@mattkime
Copy link
Contributor Author

@elasticmachine merge upstream

@mattkime
Copy link
Contributor Author

waiting for #111201 to be resolved first

@mattkime
Copy link
Contributor Author

@elasticmachine merge upstream

@mattkime mattkime changed the title Index patterns - saved object client use resolve instead of get Data views - saved object client use resolve instead of get Sep 19, 2021
@mattkime mattkime marked this pull request as ready for review September 19, 2021 20:18
@mattkime mattkime requested a review from a team as a code owner September 19, 2021 20:18
@mattkime mattkime added auto-backport Deprecated - use backport:version if exact versions are needed Feature:Data Views Data Views code and UI - index patterns before 8.0 Team:AppServices v7.16.0 v8.0.0 labels Sep 19, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-services (Team:AppServices)

@mattkime mattkime added the release_note:skip Skip the PR/issue when compiling release notes label Sep 19, 2021
ppisljar
ppisljar approved these changes Sep 20, 2021
View reviewed changes
Copy link
Member

@ppisljar ppisljar left a comment

Choose a reason for hiding this comment

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

can you please add a test for this, apart from that LGTM
note: can you please also open an issue to deduplicate this whole file into common folder ?

@mattkime
Copy link
Contributor Author

note: can you please also open an issue to deduplicate this whole file into common folder ?

I'm not sure what you mean. The error class is in common but both implementations are server and client specific. There could be a little more code reuse but I think the number of lines of code would be the same in the end.

@ppisljar
Copy link
Member

saved_objects_client_wrapper.ts in server and public look pretty much the same ?

@mattkime
Copy link
Contributor Author

saved_objects_client_wrapper.ts in server and public look pretty much the same ?

The differences are important. I'm open to specific suggestions.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
data 570 571 +1

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
data 2749 2802 +53

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
data 492.0KB 492.3KB +331.0B
Unknown metric groups

API count

id before after diff
data 3110 3188 +78

References to deprecated APIs

id before after diff
graph 98 102 +4
indexPatternManagement 299 306 +7
visTypeTimeseries 224 226 +2
total +13

History

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

@mattkime mattkime merged commit 119c742 into elastic:master Sep 21, 2021
@kibanamachine kibanamachine mentioned this pull request Sep 21, 2021
Merged
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
✅ 7.x

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Sep 21, 2021
@mattkime @kibanamachine
Data views - saved object client use resolve instead of get (elas…
e8c8777 
…tic#108637)

* so client - use resolve instead of get

Co-authored-by: Kibana Machine <[email protected]>
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Sep 23, 2021
@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Sep 23, 2021
kibanamachine added a commit that referenced this pull request Sep 23, 2021
@kibanamachine @mattkime
[7.x] Data views - saved object client use resolve instead of get (…
167fa4b 
…#108637) (#112670)

* Data views - saved object client use `resolve` instead of `get` (#108637)

* so client - use resolve instead of get

Co-authored-by: Kibana Machine <[email protected]>

* lint fix

Co-authored-by: Matthew Kime <[email protected]>
@jportner jportner mentioned this pull request Sep 27, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ppisljar ppisljar ppisljar approved these changes

Assignees
No one assigned
Labels
auto-backport Deprecated - use backport:version if exact versions are needed Feature:Data Views Data Views code and UI - index patterns before 8.0 release_note:skip Skip the PR/issue when compiling release notes v7.16.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

use savedObjectsClient.resolve for retrieving index patterns
4 participants
@mattkime @elasticmachine @ppisljar @kibanamachine