Skip to content

use KibanaClient interface instead of Client for new client interface#72388

Merged
pgayvallet merged 7 commits intoelastic:masterfrom
pgayvallet:kbn-35508-use-kbn-client
Jul 21, 2020
Merged

use KibanaClient interface instead of Client for new client interface#72388
pgayvallet merged 7 commits intoelastic:masterfrom
pgayvallet:kbn-35508-use-kbn-client

Conversation

@pgayvallet
Copy link
Copy Markdown
Contributor

@pgayvallet pgayvallet commented Jul 20, 2020

Summary

Part of #35508

Use new KibanaClient interface as base for ElasticsearchClient

Checklist

  • Documentation was added for features that require explanation or tutorials

@pgayvallet pgayvallet added v7.10.0 v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// labels Jul 20, 2020
@pgayvallet pgayvallet marked this pull request as ready for review July 20, 2020 07:41
@pgayvallet pgayvallet requested a review from a team as a code owner July 20, 2020 07:41
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Jul 20, 2020

Pinging @elastic/kibana-platform (Team:Platform)

mshustov
mshustov approved these changes Jul 20, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@mshustov mshustov left a comment

Choose a reason for hiding this comment

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

Shouldn't transport.request return TransportRequestPromise?

@pgayvallet
Copy link
Copy Markdown
Contributor Author

pgayvallet commented Jul 20, 2020

Shouldn't transport.request return TransportRequestPromise?

I would say yes, but I'll let @delvedor answer this one.

delvedor
delvedor reviewed Jul 20, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@delvedor delvedor left a comment

Choose a reason for hiding this comment

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

Yep, that's a bug. It will be fixed by elastic/elasticsearch-js#1263.

Out of curiosity, why are you omitting the helpers in the exported client?

@delvedor delvedor mentioned this pull request Jul 20, 2020
Merged
@pgayvallet
Copy link
Copy Markdown
Contributor Author

pgayvallet commented Jul 20, 2020 •
edited
Loading

Out of curiosity, why are you omitting the helpers in the exported client?

Hum, that looks like a mistake on my side. Gonna fix that

delvedor
delvedor reviewed Jul 20, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@delvedor delvedor left a comment

Choose a reason for hiding this comment

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

You should also update the transport.request definition to return TransportRequestPromise :)
https://github.com/pgayvallet/kibana/blob/7f12fb915805f90404413aa63cb3f0b231cff3c3/src/core/server/elasticsearch/client/types.ts#L40

@pgayvallet
Copy link
Copy Markdown
Contributor Author

pgayvallet commented Jul 20, 2020

Need #72448 to be merged first to change the transport.request signature, else our interface is no longer compatible and causes TS errors.

@pgayvallet
Copy link
Copy Markdown
Contributor Author

pgayvallet commented Jul 20, 2020

retest

@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Jul 21, 2020

💚 Build Succeeded

Build metrics

✅ unchanged

History

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

@pgayvallet pgayvallet merged commit 005a112 into elastic:master Jul 21, 2020
pgayvallet added a commit to pgayvallet/kibana that referenced this pull request Jul 21, 2020
@pgayvallet
use KibanaClient interface instead of Client for new client interface (…
31fe5db 
…elastic#72388)

* use KibanaClient interface instead of Client

* add back helpers to ElasticsearchClient interface

* use TransportRequestPromise for transport.request

* update generated doc
@pgayvallet pgayvallet mentioned this pull request Jul 21, 2020
Merged
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jul 21, 2020
@gmmorris
Merge branch 'master' into alerting/consumer-based-rbac
407b09a 
* master: (28 commits)
  allow some env settings for ingest manager (elastic#72544)
  Add inspector for VEGA (elastic#70941)
  chore(NA): fix grunt task for test:coverage (elastic#72539)
  Archive e2e test results in ES (elastic#72575)
  preserve 401 errors from new es client (elastic#71248)
  [SIEM][Detections] Updates text for severity and risk_score overrides  (elastic#72244)
  fixing error occurences tooltip (elastic#72425)
  use KibanaClient interface instead of Client for new client interface (elastic#72388)
  [APM] Handle ML errors (elastic#72316)
  [Discover] Improve histogram tests (elastic#72235)
  [ftr/webdriver] retry on all errors, use Rx so that timers are canceled (elastic#72540)
  [pre-req] Move .storybook to storybook; standardize files (elastic#72384)
  [Security_Solution][Resolver][Bug]: Restore breadcrumb background (elastic#72538)
  [ML] Fix annotation detector linking & delayed_data(0) (elastic#72468)
  [Security Solution][Exceptions] - Make esTypes and subType available to index patterns (elastic#72336)
  [SIEM] Uses faster wait from testing-library and removes duplicate older wait idiom (elastic#72509)
  Fix long combo box items breaking out of flex item width (elastic#72512)
  [pipeline/commitStatus] update commit status in baseline-capture job (elastic#72366)
  [Security Solution][Resolver] Update the resolver element ref on scroll events if the position of the element has changed within the page (elastic#72461)
  [Maps] auto-fit to data bounds (elastic#72129)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jul 21, 2020
@gmmorris
Merge branch 'alerting/consumer-based-rbac' into alerting/management-…
8545f64 
…feature-privileges

* alerting/consumer-based-rbac: (30 commits)
  removed uneeded tests
  expclude security wrapper in SO client passed to ActionsClient
  allow some env settings for ingest manager (elastic#72544)
  Add inspector for VEGA (elastic#70941)
  chore(NA): fix grunt task for test:coverage (elastic#72539)
  Archive e2e test results in ES (elastic#72575)
  preserve 401 errors from new es client (elastic#71248)
  [SIEM][Detections] Updates text for severity and risk_score overrides  (elastic#72244)
  fixing error occurences tooltip (elastic#72425)
  use KibanaClient interface instead of Client for new client interface (elastic#72388)
  [APM] Handle ML errors (elastic#72316)
  [Discover] Improve histogram tests (elastic#72235)
  [ftr/webdriver] retry on all errors, use Rx so that timers are canceled (elastic#72540)
  [pre-req] Move .storybook to storybook; standardize files (elastic#72384)
  [Security_Solution][Resolver][Bug]: Restore breadcrumb background (elastic#72538)
  [ML] Fix annotation detector linking & delayed_data(0) (elastic#72468)
  [Security Solution][Exceptions] - Make esTypes and subType available to index patterns (elastic#72336)
  [SIEM] Uses faster wait from testing-library and removes duplicate older wait idiom (elastic#72509)
  Fix long combo box items breaking out of flex item width (elastic#72512)
  [pipeline/commitStatus] update commit status in baseline-capture job (elastic#72366)
  ...
pgayvallet added a commit that referenced this pull request Jul 21, 2020
@pgayvallet
use KibanaClient interface instead of Client for new client interface (…
e3ed72d 
…#72388) (#72578)

* use KibanaClient interface instead of Client

* add back helpers to ElasticsearchClient interface

* use TransportRequestPromise for transport.request

* update generated doc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@delvedor delvedor delvedor left review comments

+1 more reviewer

@mshustov mshustov mshustov approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

release_note:skip Skip the PR/issue when compiling release notes Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// v7.10.0 v8.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@pgayvallet @elasticmachine @kibanamachine @mshustov @delvedor