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

Remove legacy elasticsearch client usage from the reporting plugin #97184

Merged
merged 1 commit into from
Apr 16, 2021

Conversation

dokmic
Copy link
Contributor

@dokmic dokmic commented Apr 14, 2021

Summary

Removing legacy elasticsearch client usage from the reporting plugin.

Resolves #95375.

Checklist

For maintainers

@dokmic dokmic added review Feature:Reporting Reporting (PDF, CSV, ..) feature v8.0.0 Team:AppServices release_note:skip Skip the PR/issue when compiling release notes v7.13.0 labels Apr 14, 2021
@dokmic dokmic requested a review from tsullivan April 14, 2021 20:29
@elasticmachine
Copy link
Contributor

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

@dokmic dokmic marked this pull request as draft April 14, 2021 20:52
@dokmic dokmic marked this pull request as ready for review April 15, 2021 11:59
import { CancellationToken } from '../../../../common';
import { LevelLogger } from '../../../lib';
import { ScrollConfig } from '../../../types';

export type EndpointCaller = (method: string, params: object) => Promise<SearchResponse<any>>;
type SearchResponse = UnwrapPromise<ReturnType<ElasticsearchClient['search']>>;
type SearchRequest = Required<Parameters<ElasticsearchClient['search']>>[0];
Copy link
Member

Choose a reason for hiding this comment

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

TIL about Parameters<Type>

this.indexPrefix = config.get('index');
this.indexInterval = config.get('queue', 'indexInterval');
this.logger = logger.clone(['store']);
this.queueTimeoutMins = Math.ceil(numberToDuration(config.get('queue', 'timeout')).asMinutes());
}

private async getClient() {
if (!this.client) {
({ asInternalUser: this.client } = await this.reportingCore.getEsClient());
Copy link
Member

Choose a reason for hiding this comment

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

TIL: you can destructure into an instance variable

this.logger.error(error);

throw error;
}
Copy link
Member

Choose a reason for hiding this comment

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

++ to rewriting this to use await

Copy link
Member

@tsullivan tsullivan left a comment

Choose a reason for hiding this comment

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

This is great! I love the cleanups of using more await instead of promise chaining.

I left one comment for conciseness, but I'd love to get this merged ASAP.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

@dokmic dokmic merged commit 15e8ca1 into elastic:master Apr 16, 2021
dokmic added a commit to dokmic/kibana that referenced this pull request Apr 16, 2021
…ng plugin (elastic#97184)

# Conflicts:
#	x-pack/plugins/reporting/server/export_types/csv/execute_job.test.ts
#	x-pack/plugins/reporting/server/export_types/csv/execute_job.ts
#	x-pack/plugins/reporting/server/lib/store/store.test.ts
#	x-pack/plugins/reporting/server/lib/store/store.ts
#	x-pack/plugins/reporting/server/plugin.ts
jloleysens added a commit to jloleysens/kibana that referenced this pull request Apr 19, 2021
…te-legacy-es-client

* 'master' of github.com:elastic/kibana: (102 commits)
  [Exploratory view] integerate page views to exploratory view (elastic#97258)
  Fix typo in license_api_guard README name and import http server mocks from public interface (elastic#97334)
  Avoid mutating KQL query when validating it (elastic#97081)
  Add description as title on tag badge (elastic#97109)
  Remove legacy ES client usages in `home` and `xpack_legacy` (elastic#97359)
  [Fleet] Finer-grained error information from install/upgrade API (elastic#95649)
  Rule registry bundle size (elastic#97251)
  [Partial Results] Move other bucket into Search Source (elastic#96384)
  [Dashboard] Makes lens default editor for creating new panels (elastic#96181)
  skip flaky suite (elastic#97387)
  [Asset Management] Agent picker follow up (elastic#97357)
  skip flaky suite (elastic#97382)
  [Security Solutions] Fixes flake with cypress tests (elastic#97329)
  skip flaky suite (elastic#97355)
  Skip test to try and stabilize master
  minimize number of so fild asserted in tests. it creates flakines when implementation details change (elastic#97374)
  [Search Sessions] Client side search cache (elastic#92439)
  [SavedObjects] Add aggregations support (elastic#96292)
  [Reporting] Remove legacy elasticsearch client usage from the reporting plugin (elastic#97184)
  [kbnClient] fix basePath handling and export reponse type (elastic#97277)
  ...

# Conflicts:
#	x-pack/plugins/watcher/server/lib/license_pre_routing_factory/license_pre_routing_factory.ts
#	x-pack/plugins/watcher/server/plugin.ts
#	x-pack/plugins/watcher/server/routes/api/indices/register_get_route.ts
#	x-pack/plugins/watcher/server/routes/api/license/register_refresh_route.ts
#	x-pack/plugins/watcher/server/routes/api/register_list_fields_route.ts
#	x-pack/plugins/watcher/server/routes/api/register_load_history_route.ts
#	x-pack/plugins/watcher/server/routes/api/settings/register_load_route.ts
#	x-pack/plugins/watcher/server/routes/api/watch/action/register_acknowledge_route.ts
#	x-pack/plugins/watcher/server/routes/api/watch/register_activate_route.ts
#	x-pack/plugins/watcher/server/routes/api/watch/register_deactivate_route.ts
#	x-pack/plugins/watcher/server/routes/api/watch/register_delete_route.ts
#	x-pack/plugins/watcher/server/routes/api/watch/register_execute_route.ts
#	x-pack/plugins/watcher/server/routes/api/watch/register_history_route.ts
#	x-pack/plugins/watcher/server/routes/api/watch/register_load_route.ts
#	x-pack/plugins/watcher/server/routes/api/watch/register_save_route.ts
#	x-pack/plugins/watcher/server/routes/api/watch/register_visualize_route.ts
#	x-pack/plugins/watcher/server/routes/api/watches/register_delete_route.ts
#	x-pack/plugins/watcher/server/routes/api/watches/register_list_route.ts
#	x-pack/plugins/watcher/server/shared_imports.ts
#	x-pack/plugins/watcher/server/types.ts
dokmic added a commit to dokmic/kibana that referenced this pull request Apr 19, 2021
dokmic added a commit to dokmic/kibana that referenced this pull request Apr 19, 2021
dokmic added a commit to dokmic/kibana that referenced this pull request Apr 19, 2021
@dokmic dokmic deleted the feature/95375 branch April 20, 2021 08:01
madirey pushed a commit to madirey/kibana that referenced this pull request May 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Reporting Reporting (PDF, CSV, ..) feature release_note:skip Skip the PR/issue when compiling release notes review v7.13.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Reporting] Switch to the new Elasticsearch client
4 participants