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

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

Merged
merged 4 commits into from
Nov 13, 2019

Conversation

lukasolson
Copy link
Member

@lukasolson lukasolson commented Oct 25, 2019

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
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
Contributor

💚 Build Succeeded

@stacey-gammon
Copy link
Contributor

@elasticmachine merge upstream

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

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. 🤷‍♀

Copy link
Member Author

@lukasolson lukasolson Oct 30, 2019

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.

Copy link
Contributor

@stacey-gammon stacey-gammon left a comment

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
Contributor

💚 Build Succeeded

@lukasolson
Copy link
Member Author

@elasticmachine merge upstream

@elasticmachine
Copy link
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
Member 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
Contributor

💚 Build Succeeded

@lukasolson lukasolson deleted the searchPreference branch December 2, 2019 22:36
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