Skip to content

Conversation

@lukasolson
Copy link
Contributor

Summary

Resolves #75356.

This PR removes the long-running query pop-up that shows up after 15 seconds and prompts the user to run beyond timeout. This will be replaced by a configuration value that administrators can control for async search requests explained here: #75321

Checklist

  • Functional tests

(Release notes will be added in the PR that adds the new advanced setting.)

@lukasolson lukasolson added Feature:Search Querying infrastructure in Kibana v8.0.0 Team:AppArch release_note:skip Skip the PR/issue when compiling release notes v7.10.0 labels Aug 19, 2020
@lukasolson lukasolson requested a review from a team as a code owner August 19, 2020 02:11
@lukasolson lukasolson self-assigned this Aug 19, 2020
@elasticmachine
Copy link
Contributor

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

@lizozom
Copy link
Contributor

lizozom commented Aug 20, 2020

@elasticmachine merge upstream

@lizozom lizozom closed this Aug 20, 2020
@elasticmachine
Copy link
Contributor

ignoring request to update branch, pull request is closed

@lizozom lizozom reopened this Aug 20, 2020
@lizozom
Copy link
Contributor

lizozom commented Aug 20, 2020

@elasticmachine merge upstream

@lukasolson lukasolson force-pushed the search-remove-popup branch from 666cd7f to c1217b0 Compare August 20, 2020 18:27
@lizozom
Copy link
Contributor

lizozom commented Aug 23, 2020

@elasticmachine merge upstream

const { signal: timeoutSignal } = timeoutController;
const timeout$ = timer(this.requestTimeout);
const timeout$ =
this.requestTimeout != null && this.requestTimeout > 0 ? timer(this.requestTimeout) : NEVER;
Copy link
Contributor

Choose a reason for hiding this comment

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

//nit

this.requestTimeout != null && this.requestTimeout > 0 ➡️ !this.requestTimeout ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure I understand, could you provide the code snippet?

Copy link
Contributor

@lizozom lizozom left a comment

Choose a reason for hiding this comment

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

I think we can go ahead and merge it, adding tests once the delayed agg is available.
LGTM

@lizozom
Copy link
Contributor

lizozom commented Aug 23, 2020

After thinking some more about it, I think that the search interceptor should still be responsible for showing the timeout toast.
The reasoning behind is that we're going to have different types of notifications per-license and permissions, and I think it could be convenient if those were handled in one place.

Alternatively, we could provide a component that is called by the solution and shows the correct message.

Lets talk about it.

@lizozom lizozom removed the request for review from a team September 8, 2020 17:08
@lizozom
Copy link
Contributor

lizozom commented Sep 9, 2020

@elasticmachine merge upstream

@lizozom lizozom removed the request for review from a team September 10, 2020 07:42
// error notification per session.
protected showTimeoutError = debounce(
(e: Error) => {
const message = this.application.capabilities.advancedSettings?.save
Copy link
Contributor

Choose a reason for hiding this comment

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

All strings need to be translated

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

@kbn/optimizer bundle module count

id value diff baseline
data 541 -1 542
dataEnhanced 20 -1 21
total -2

page load bundle size

id value diff baseline
data 1.4MB -5.0KB 1.4MB
dataEnhanced 177.1KB -1.6KB 178.8KB
total -6.6KB

History

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

@lukasolson lukasolson merged commit b99d8af into elastic:master Sep 13, 2020
lukasolson added a commit to lukasolson/kibana that referenced this pull request Sep 13, 2020
* [Search] Remove long-running query pop-up

* Don't timeout if requestTimeout isn't configured

* Remove unused kibanaUtils

* Remove unused kibanaReact

* Re-add reference to kibanaUtils

* Remove unused translations and update documentation

* Add new x-pack advanced setting searchTimeout and use it in the EnhancedSearchInterceptor

* docs

* Re-add toast when queries time out

* Fix types

* Update error message with capabilities

* Update docs

* Update docs

* Move search server routes into a directory.

* Add internal/_msearch route.

* Remove legacy search API, rewrite default search strategy to use internal route.

* Remove legacy es_client code.

* Handle msearch options on server.

* Remove elasticsearch-browser dependency.

* Update generated docs.

* Rely on server timeout in OSS (?)
Use UI setting in xpack.

* Rename function

* Add features to dependencies

* Undefined check

* doc

* Code review fixes

* code review

* doc

* loading count

* simplify code review and fix jest tets

* type check

* Remove esShard from client

* cleanup request parameters from FE

* doc

* doc

* Align request parameters on server,
Remove leftover parameters from client
Shim responses for search and msearch routes

* docs
Stop using toSnakeCase
Updates jest tests

* add management docs

* docs

* Remove import

* Break circular dep + fix msearch test

* Remove deleted type

* Fix jest

* Bring toSnakeCase back

* docs

* fix jest

* Add new x-pack advanced setting searchTimeout and use it in the EnhancedSearchInterceptor

* docs

* Rely on server timeout in OSS (?)
Use UI setting in xpack.

* Rename function

* doc

* Remove esShard from client

* cleanup request parameters from FE

* doc

* doc

* Align request parameters on server,
Remove leftover parameters from client
Shim responses for search and msearch routes

* docs
Stop using toSnakeCase
Updates jest tests

* add management docs

* docs

* Remove import

* Break circular dep + fix msearch test

* Remove deleted type

* Fix jest

* Bring toSnakeCase back

* docs

* fix jest

* Fix merge

* Fix types

* Allow timeout to be undefined

* Fix jest test

* Upldate docs

* Fix msearch jest

* Merge correction

* docs

* Fix rollup search merge

* Fix merge

* Use i18n

Co-authored-by: Liza K <liza.katz@elastic.co>
Co-authored-by: Luke Elmers <luke.elmers@elastic.co>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
lukasolson added a commit that referenced this pull request Sep 13, 2020
* [Search] Remove long-running query pop-up

* Don't timeout if requestTimeout isn't configured

* Remove unused kibanaUtils

* Remove unused kibanaReact

* Re-add reference to kibanaUtils

* Remove unused translations and update documentation

* Add new x-pack advanced setting searchTimeout and use it in the EnhancedSearchInterceptor

* docs

* Re-add toast when queries time out

* Fix types

* Update error message with capabilities

* Update docs

* Update docs

* Move search server routes into a directory.

* Add internal/_msearch route.

* Remove legacy search API, rewrite default search strategy to use internal route.

* Remove legacy es_client code.

* Handle msearch options on server.

* Remove elasticsearch-browser dependency.

* Update generated docs.

* Rely on server timeout in OSS (?)
Use UI setting in xpack.

* Rename function

* Add features to dependencies

* Undefined check

* doc

* Code review fixes

* code review

* doc

* loading count

* simplify code review and fix jest tets

* type check

* Remove esShard from client

* cleanup request parameters from FE

* doc

* doc

* Align request parameters on server,
Remove leftover parameters from client
Shim responses for search and msearch routes

* docs
Stop using toSnakeCase
Updates jest tests

* add management docs

* docs

* Remove import

* Break circular dep + fix msearch test

* Remove deleted type

* Fix jest

* Bring toSnakeCase back

* docs

* fix jest

* Add new x-pack advanced setting searchTimeout and use it in the EnhancedSearchInterceptor

* docs

* Rely on server timeout in OSS (?)
Use UI setting in xpack.

* Rename function

* doc

* Remove esShard from client

* cleanup request parameters from FE

* doc

* doc

* Align request parameters on server,
Remove leftover parameters from client
Shim responses for search and msearch routes

* docs
Stop using toSnakeCase
Updates jest tests

* add management docs

* docs

* Remove import

* Break circular dep + fix msearch test

* Remove deleted type

* Fix jest

* Bring toSnakeCase back

* docs

* fix jest

* Fix merge

* Fix types

* Allow timeout to be undefined

* Fix jest test

* Upldate docs

* Fix msearch jest

* Merge correction

* docs

* Fix rollup search merge

* Fix merge

* Use i18n

Co-authored-by: Liza K <liza.katz@elastic.co>
Co-authored-by: Luke Elmers <luke.elmers@elastic.co>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Liza K <liza.katz@elastic.co>
Co-authored-by: Luke Elmers <luke.elmers@elastic.co>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
jloleysens added a commit to jloleysens/kibana that referenced this pull request Sep 14, 2020
…s-for-710

* 'master' of github.com:elastic/kibana: (65 commits)
  Separate url forwarding logic and legacy services (elastic#76892)
  Bump yargs-parser to v13.1.2+ (elastic#77009)
  [Ingest Manager] Shared Fleet agent policy action (elastic#76013)
  [Search] Re-add support for aborting when a connection is closed (elastic#76470)
  [Search] Remove long-running query pop-up (elastic#75385)
  [Monitoring] Fix UI error when alerting is not available (elastic#77179)
  do not log plugin id format warning in dist mode (elastic#77134)
  [ML] Improving client side error handling (elastic#76743)
  [Alerting][Connectors] Refactor IBM Resilient: Generic Implementation (phase one) (elastic#74357)
  [Docs] some basic searchsource api docs (elastic#77038)
  add  cGroupOverrides to the legacy config (elastic#77180)
  Change saved object bulkUpdate to work across multiple namespaces (elastic#75478)
  [Security Solution][Resolver] Replace Selectable popover with badges (elastic#76997)
  Removing ml-state index from archive (elastic#77143)
  [Security Solution] Add unit tests for histograms (elastic#77081)
  [Lens] Filters aggregation  (elastic#75635)
  [Enterprise Search] Update WS Overview logic to use new config data (elastic#77122)
  Cleanup type output before building new types (elastic#77211)
  [Security Solution] Use safe type in resolver backend (elastic#76969)
  Use proper lodash syntax (elastic#77105)
  ...

# Conflicts:
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/node_allocation.tsx
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature:Search Querying infrastructure in Kibana release_note:skip Skip the PR/issue when compiling release notes v7.10.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Search] Remove run beyond timeout toast

7 participants