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

Clean up search service #53766

Merged
merged 65 commits into from
Jan 21, 2020
Merged

Conversation

lizozom
Copy link
Contributor

@lizozom lizozom commented Dec 23, 2019

Summary

Partially resolves #46934.

As #44302 won't allow us to switch over to the new service, this PR contains all the changes necessary to make the switch, but without actually making it.

New search service & further improvements

This PR contains tested infrastructure for the new data.searchService.

  • Replace the angular es service in default_search_strategy.ts with a non-angular one and removes any angular \ old platform dependencies from search source and default search strategy, to allow moving them to NP.
  • LoadingIndicator handling - show loading indicator while searching
  • Exposing error codes and info from the search route
  • Changing the way getPainlessError is processed due to slightly different response from new endpoint.
  • Allow AbortError to be thrown directly from http service @elastic/kibana-platform for your review
  • Triggering a digest cycle in maps, as its now not being triggered automatically by elasticsearch client.
  • Minor fix for test/functional/apps/discover/_errors.js that allows running it independently

Checklist

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

For maintainers

@lizozom lizozom self-assigned this Dec 23, 2019
@elasticmachine
Copy link
Contributor

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

@kibanamachine
Copy link
Contributor

💔 Build Failed

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

@kibanamachine
Copy link
Contributor

💔 Build Failed

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

@kibanamachine
Copy link
Contributor

💔 Build Failed

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

@kibanamachine
Copy link
Contributor

💔 Build Failed

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

@kibanamachine
Copy link
Contributor

💔 Build Failed

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

Comment on lines +53 to +54
const customPromiseThingy = esClient[method](args);
const { abort } = customPromiseThingy;
Copy link
Member

Choose a reason for hiding this comment

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

🤣

Copy link
Contributor Author

@lizozom lizozom Jan 17, 2020

Choose a reason for hiding this comment

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

This one is @spalger approved 🤣

@@ -51,6 +53,9 @@ declare module 'kibana/public' {

export interface ISearchStart {
search: ISearchGeneric;
__LEGACY: {
esClient: any;
Copy link
Member

Choose a reason for hiding this comment

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

Does this need to be any? I think we have a type for this (APICaller).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lizozom
Copy link
Contributor Author

lizozom commented Jan 17, 2020

@elasticmachine merge upstream

@lizozom lizozom added v7.7.0 and removed v7.6.0 labels Jan 17, 2020
@elastic elastic deleted a comment from elasticmachine Jan 18, 2020
@lizozom lizozom requested a review from lukasolson January 20, 2020 10:34
@lizozom
Copy link
Contributor Author

lizozom commented Jan 20, 2020

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@lizozom lizozom added the review label Jan 21, 2020
@lizozom lizozom merged commit f265961 into elastic:master Jan 21, 2020
lizozom pushed a commit to lizozom/kibana that referenced this pull request Jan 21, 2020
* deprecate msearch

* Missing export

* adjust tests, revert loading method of esaggs/boot

* getInjectedMetadata

* Fix jest tests

* update default strategy abort test

* notice update

* Allow running discover errors test independently

* Remove batchSearches

* Detect painless script error

* don't show notifications for aborted requests

* Fix jest tests

* Restore loader indicator

* Decreace loading count on error

* update search test

* Trigger digest after fetching fresh index patterns

* Revert isEqual

* accurate revert

* Return full error details to client from search endpoint

* Re-throw AbortError from http when user aborts request.

* fix typo

* typo

* Adjust routes jest test

* Restore msearch using a separate es connection

* typescript fixes

* set http service mock

* Move es client to dat aplugin, for follow up PR

* Add karma mock

* krma mock

* fix tests

* ts

* Pass in version dynamically

* add headers to esClient host

* Restored fetch soon test
Use tap for loadingCount side effects

* Cleanup search params

* Cleanup search params test

* Revert "Cleanup search params"

This reverts commit ca9dea0.

* Revert "Cleanup search params test"

This reverts commit 30b9478.

* Revert code to use old es client until  elastic#44302 is resolved

* Revert changes to getPainlessError

* Fix jest test

* Refactor esClient to trigger loadingIndicator

* fixing tests

* use esClient from searchService

* git remove comment

* fix jest

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
lizozom pushed a commit that referenced this pull request Jan 21, 2020
* deprecate msearch

* Missing export

* adjust tests, revert loading method of esaggs/boot

* getInjectedMetadata

* Fix jest tests

* update default strategy abort test

* notice update

* Allow running discover errors test independently

* Remove batchSearches

* Detect painless script error

* don't show notifications for aborted requests

* Fix jest tests

* Restore loader indicator

* Decreace loading count on error

* update search test

* Trigger digest after fetching fresh index patterns

* Revert isEqual

* accurate revert

* Return full error details to client from search endpoint

* Re-throw AbortError from http when user aborts request.

* fix typo

* typo

* Adjust routes jest test

* Restore msearch using a separate es connection

* typescript fixes

* set http service mock

* Move es client to dat aplugin, for follow up PR

* Add karma mock

* krma mock

* fix tests

* ts

* Pass in version dynamically

* add headers to esClient host

* Restored fetch soon test
Use tap for loadingCount side effects

* Cleanup search params

* Cleanup search params test

* Revert "Cleanup search params"

This reverts commit ca9dea0.

* Revert "Cleanup search params test"

This reverts commit 30b9478.

* Revert code to use old es client until  #44302 is resolved

* Revert changes to getPainlessError

* Fix jest test

* Refactor esClient to trigger loadingIndicator

* fixing tests

* use esClient from searchService

* git remove comment

* fix jest

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Search service] Use new search service in SearchSource
8 participants