-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[context view] Use courier when querying the context #11127
[context view] Use courier when querying the context #11127
Conversation
I hoped to gain |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great! left one suggestion that you can take or leave as you see fit. I also noticed reverseQuerySort is no longer used. Not sure if you still have plans for it, or if you'd like to delete it.
const queryParameterActions = Private(QueryParameterActionsProvider); | ||
const queryActions = Private(QueryActionsProvider); | ||
|
||
// this is apparently the "canonical" way to disable the time picker | ||
timefilter.enabled = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
huh... that concerning. As you say, this seems to be the way to do it at the moment though.
@@ -16,6 +16,7 @@ export default function FetchStrategyForSearch(Private, Promise, timefilter, kbn | |||
*/ | |||
reqsFetchParamsToBody: function (reqsFetchParams) { | |||
const indexToListMapping = {}; | |||
const timeBounds = timefilter.enabled ? timefilter.getBounds() : null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like there's a getActiveBounds method you could use on the timefilter.
|
||
return async function fetchAnchor(indexPattern, uid, sort) { | ||
const searchSource = new SearchSource() | ||
.inherits(false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm trying to figure out what exactly this line accomplishes... Does the SearchSource
inherit something else by default?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it takes a while to trace the effect. Ultimately it disables the default inheritance from the rootSearchSource
:
kibana/src/ui/public/courier/data_source/search_source.js
Lines 151 to 156 in 9043476
SearchSource.prototype.getParent = function (onlyHardLinked) { | |
const self = this; | |
if (self._parent === false) return; | |
if (self._parent) return self._parent; | |
return onlyHardLinked ? undefined : Private(rootSearchSource).get(); | |
}; |
This is how the the timefilter values get mixed into the request, which I needed to disable for the context query.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Great improvement.
ccc60f5
to
69e2a8a
Compare
A clojure trace |
@weltenwort looks like jenkins had no ear for your nice poem ;) |
jenkins, listen to felix and test this |
Instead of using a separate code path for querying the anchor and context documents, the context view actions now use a normal `SearchSource` and thereby scripted fields, source filtering and other features of a `SearchSource` without duplicated code. To facilitate this the `SearchSource` gained the `searchAfter` field, which is turned into a `search_after` clause on the request. Additionally, the `FetchStrategyForSearch` now honors the `timefilter.enabled` flag. When this flag is enabled, the index list will be expanded with respect to the current time interval configured in the `timefilter` (previous behaviour). When this flag is disabled, the `timefilter` values will be ignored when expanding the index list.
69e2a8a
to
bf8ddb5
Compare
…1127) Instead of using a separate code path for querying the anchor and context documents, the context view actions now use a normal `SearchSource` and thereby scripted fields, source filtering and other features of a `SearchSource` without duplicated code. To facilitate this the `SearchSource` gained the `searchAfter` field, which is turned into a `search_after` clause on the request. Additionally, the `FetchStrategyForSearch` now honors the `timefilter.enabled` flag. When this flag is enabled, the index list will be expanded with respect to the current time interval configured in the `timefilter` (previous behaviour). When this flag is disabled, the `timefilter` values will be ignored when expanding the index list.
Backports PR #11127 with adaptions to 5.x (see below) * [5.x] [context view] Use courier when querying the context (#11127) Instead of using a separate code path for querying the anchor and context documents, the context view actions now use a normal `SearchSource` and thereby scripted fields, source filtering and other features of a `SearchSource` without duplicated code. To facilitate this the `SearchSource` gained the `searchAfter` field, which is turned into a `search_after` clause on the request. Additionally, the `FetchStrategyForSearch` now honors the `timefilter.enabled` flag. When this flag is enabled, the index list will be expanded with respect to the current time interval configured in the `timefilter` (previous behaviour). When this flag is disabled, the `timefilter` values will be ignored when expanding the index list. * Undo named exports fix until rollout has been backported
Instead of using a separate code path for querying the anchor and
context documents, the context view actions now use a normal
SearchSource
and thereby support scripted fields, source filtering and otherfeatures of a
SearchSource
without duplicated code.To facilitate this the
SearchSource
gained thesearchAfter
field,which is turned into a
search_after
clause on the request.Additionally, the
FetchStrategyForSearch
now honors thetimefilter.enabled
flag. When this flag is enabled, the index listwill be expanded with respect to the current time interval configured in
the
timefilter
(previous behaviour). When this flag is disabled, thetimefilter
values will be ignored when expanding the index list.