Skip to content

[Search service] Add support for ES request preference - #49424

Merged
lukasolson merged 4 commits into
elastic:masterfrom
lukasolson:searchPreference
Nov 13, 2019
Merged

lukasolson merged 4 commits into
elastic:masterfrom
lukasolson:searchPreference

Conversation

@lukasolson

@lukasolson lukasolson commented Oct 25, 2019 •

Copy link
Copy Markdown
Contributor

Summary

Adds support to the new search service for setting the Elasticsearch request preference based on the courier:setRequestPreference and courier:customRequestPreference UI settings.

Checklist

@lukasolson lukasolson added Feature:Search Querying infrastructure in Kibana v8.0.0 Team:AppArch v7.6.0 labels Oct 25, 2019
@lukasolson
lukasolson requested a review from a team as a code owner October 25, 2019 22:04
@lukasolson lukasolson self-assigned this Oct 25, 2019
@elasticmachine

Copy link
Copy Markdown
Contributor

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

@lukasolson lukasolson added the release_note:skip Skip the PR/issue when compiling release notes label Oct 25, 2019
@elasticmachine

Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@stacey-gammon

Copy link
Copy Markdown

@elasticmachine merge upstream

search: (request, options) =>
search(
search: (request, options) => {
if (typeof request.params.preference === 'undefined') {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Would if (request.params.preference === undefined) { work? a bit shorter. 🤷‍♀

@lukasolson lukasolson Oct 30, 2019 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's mostly out of habit because you can technically redefine undefined:

(undefined => {
  console.log(undefined); // 'foo'
})('foo')

But yeah, it shouldn't make any difference in this situation.

@stacey-gammon stacey-gammon left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

did not pull down and test but looks straightforward. Glad adding in settings worked out well.

I forget how the async es_search_strategy was set up, but we'll probably want to see how we aren't duplicating settings in that one. I wonder if we could hold a reference to the OSS es_search strategy after overwriting it, and re-use it, sending in a param for the server side strategy (so sync -> async) or something. 🤷‍♀ can figure that out later, for now, code lgtm.

@elasticmachine

Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@lukasolson

Copy link
Copy Markdown
Contributor Author

@elasticmachine merge upstream

@elasticmachine

Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@lukasolson
lukasolson merged commit 013f3eb into elastic:master Nov 13, 2019
lukasolson added a commit to lukasolson/kibana that referenced this pull request Nov 13, 2019
* Add support for ES preference

* Fix name of test
@lukasolson

Copy link
Copy Markdown
Contributor Author

7.x (7.6.0): 384b037

jloleysens added a commit to jloleysens/kibana that referenced this pull request Nov 14, 2019
* 'master' of github.com:elastic/kibana: (27 commits)
  [Rollup] Fix for clone job workflow (elastic#50501)
  Empty message "No data available" for Labels and User metadata sections missing (elastic#49846)
  [APM] Duration by Country map doesn't take `transactionName` into account (elastic#50315)
  Remove react references from core `Notifications` apis (elastic#49573)
  Updated APM Indices endpoints to use the SavedObjectsClient from the legacy request context, and set the apm-indices schema object to be namspace-agnostic
  [Metrics UI] Calculate interval based on the dataset's period (elastic#50194)
  chore(NA): add new platform discovered plugins as entry points to check for dependencies on clean dll tasks (elastic#50610)
  [Telemetry] change of optin status telemetry (elastic#50158)
  [SIEM][Detection Engine] REST API Additions (elastic#50514)
  [DOCS] Removes dashboard-only mode doc (elastic#50441)
  [Filters] Fix operator overflowing out popover (elastic#50030)
  Change telemetry optIn to default to true (elastic#50490)
  [Maps] make grid rectangles the default symbolization for geo grid source (elastic#50169)
  Allow registered applications to hide Kibana chrome (elastic#49795)
  Upgrade EUI to v14.9.0 (elastic#49678)
  [Metrics UI] Convert layouts to use React components (elastic#49134)
  [Search service] Add support for ES request preference (elastic#49424)
  [Newsfeed/Lint] fix chained fn lint (elastic#50515)
  [Monitoring] Fix logstash pipelines page in multi-cluster environment (elastic#50166)
  [SIEM] Events viewer fixes (elastic#50175)
  ...
@elasticmachine

Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@lukasolson
lukasolson deleted the searchPreference branch December 2, 2019 22:36
patrykkopycinski pushed a commit to patrykkopycinski/kibana that referenced this pull request May 6, 2026
* Add support for ES preference

* Fix name of test
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.6.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants