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

Replace data plugin kfetch dependency with core.http service #45498

Merged
merged 7 commits into from
Sep 16, 2019
Merged

Replace data plugin kfetch dependency with core.http service #45498

merged 7 commits into from
Sep 16, 2019

Conversation

lizozom
Copy link
Contributor

@lizozom lizozom commented Sep 12, 2019 •
edited
Loading

Summary

Part of #44377

  • Replace data plugin kfetch dependency on core.http service
  • Deprecate IndexPatternsProvider

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@lizozom lizozom requested a review from a team as a code owner September 12, 2019 08:56
@lizozom lizozom self-assigned this Sep 12, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-arch

@elasticmachine
Copy link
Contributor

💔 Build Failed

@azasypkin
Copy link
Member

ACK: will review security related change today

@legrego legrego self-requested a review September 12, 2019 12:50
@lizozom lizozom requested review from a team as code owners September 12, 2019 13:01
@lizozom lizozom force-pushed the newplatform/data-plugin/remove-kfetch-2 branch from 0066421 to fc1e8ff Compare September 12, 2019 13:05
@lizozom lizozom removed the request for review from a team September 12, 2019 13:05
legrego
legrego approved these changes Sep 12, 2019
View reviewed changes
Copy link
Member

@legrego legrego left a comment

Choose a reason for hiding this comment

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

Security changes LGTM!

This was referenced Sep 12, 2019
Closed
Closed
@lizozom lizozom changed the title Replace data plugin kfetch dependency on core.http service Replace data plugin kfetch dependency with core.http service Sep 12, 2019
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

ppisljar
ppisljar approved these changes Sep 16, 2019
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.

LGTM

src/legacy/core_plugins/data/public/index_patterns/index_patterns/index_patterns.ts Outdated
@@ -105,7 +112,7 @@ export class IndexPatterns {
return null;
};

get = (id?: string) => {
get = (id?: string | null) => {
Copy link
Member

Choose a reason for hiding this comment

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

id? already allows this not to be set, why do we need null ?

Merge branch 'newplatform/data-plugin/remove-kfetch-2' of github.com:…
4d98b26 
…lizozom/kibana into newplatform/data-plugin/remove-kfetch-2
@lizozom lizozom mentioned this pull request Sep 16, 2019
Merged
7 tasks
@lizozom lizozom requested a review from a team as a code owner September 16, 2019 11:32
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

peteharverson
peteharverson approved these changes Sep 16, 2019
View reviewed changes
Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

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

ML edit LGTM

@lizozom lizozom merged commit 4c53fc0 into elastic:master Sep 16, 2019
@lizozom lizozom mentioned this pull request Sep 16, 2019
Merged
@wylieconlon wylieconlon mentioned this pull request Sep 16, 2019
Merged
@streamich streamich mentioned this pull request Sep 27, 2019
Closed
10 tasks
@lizozom lizozom deleted the newplatform/data-plugin/remove-kfetch-2 branch November 14, 2019 13:06
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

@peteharverson peteharverson peteharverson approved these changes

@legrego legrego legrego approved these changes

@mattkime mattkime Awaiting requested review from mattkime

@lukasolson lukasolson Awaiting requested review from lukasolson

Assignees

@lizozom lizozom

Labels
Feature:NP Migration release_note:skip Skip the PR/issue when compiling release notes review v7.5.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@lizozom @elasticmachine @azasypkin @legrego @peteharverson @ppisljar