-
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
[ML] Replace use of rest_total_hits_as_int with track_total_hits #78423
[ML] Replace use of rest_total_hits_as_int with track_total_hits #78423
Conversation
Pinging @elastic/ml-ui (:ml) |
@elasticmachine merge upstream |
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.
Code LGTM.
I added a comment about removing lodash's get
in the places where we're modifying them.
}) | ||
.then((resp) => { | ||
// Because of the sampling, results of metricFunctions which use sum or count | ||
// can be significantly skewed. Taking into account totalHits we calculate a | ||
// a factor to normalize results for these metricFunctions. | ||
const totalHits = get(resp, ['hits', 'total'], 0); | ||
const totalHits = get(resp, ['hits', 'total', 'value'], 0); |
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 might be worth taking the opportunity to remove get
in these cases, as we're touching this code.
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 ⚡
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
42b96d5
to
ecd38dd
Compare
💚 Build SucceededMetrics [docs]async chunks size
History
To update your PR or re-run it, just comment with: |
…stic#78423) * [ML] Replace use of rest_total_hits_as_int with track_total_hits * [ML] Replace use of lodash get for obtaining hits total * [ML] Update mocks to use new total hits format
) (#78909) * [ML] Replace use of rest_total_hits_as_int with track_total_hits * [ML] Replace use of lodash get for obtaining hits total * [ML] Update mocks to use new total hits format
…aly-detection-partition-field * 'master' of github.com:elastic/kibana: (37 commits) [UiActions] Don't throw an error if there are no compatible actions to execute (elastic#78917) [Observability] Kibana nav when docked overlaps the content of the pages. (elastic#78593) Invalid `searchSourceJSON` causes saved object migration to fail (elastic#78535) update vega version (elastic#78390) Fix warning text doesn't get displayed on filters with custom filter name (elastic#78617) [ILM] Data tier notices should reflect tier preferences (elastic#78398) [APM] Service Map: `Not Defined` option doesn't work properly (elastic#77757) Improve invalid field error message at search.aggs.param_types.field (elastic#78587) Remove isDeprecated flag on visType (elastic#78820) Remove unused elasticsearch.preserverHost setting (elastic#78608) Fix condition and adjust tests (elastic#78898) [UX] Add percentile selector (elastic#78562) [ML] Replace use of rest_total_hits_as_int with track_total_hits (elastic#78423) expression service docs (elastic#78774) [Functional] Wait for the page to load and then click the new vis button (elastic#78725) [TSVB] No data in visualizations with annotations (elastic#78794) [kbn/ui-shared-deps] track asset sizes (elastic#78718) delete target before building (elastic#78665) Move indexPattern.popularizeField into discover (elastic#77668) [Security Solution][Resolver]Add backdrop to pills (elastic#78625) ...
Summary
Replaces all usage of the
rest_total_hits_as_int
parameter from search queries in the ML and Transforms plugins. Searches now usetrack_total_hits
and obtain the value from thehits.total
object in the search response.rest_total_hits_as_int
is being removed in 8.0.Note most places in the code do not need the exact total of hits, and so use the default
track_total_hits
(10000). For the places where the exact number is needed (e.g. in the data visualizer)track_total_hits: true
is explicitly set in the request.Fixes #71852