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

[ML] Add rest_total_hit_as_int where total hits is required #26421

Merged
merged 1 commit into from
Dec 4, 2018

Conversation

peteharverson
Copy link
Contributor

Summary

Implements step 1 of #26356 for the ML UI, adding rest_total_hits_as_int=true to all requests to Elasticsearch where we need to get hits.total.

After 7.0 has been released, steps 2 and 3 of #26356 should be addressed in a follow-up PR. Most of the ML uses of hits.total can probably either be switched to use the estimated hits count, or be removed altogether.

Checklist

N/A

@peteharverson peteharverson added review non-issue Indicates to automation that a pull request should not appear in the release notes v7.0.0 :ml Feature:Anomaly Detection ML anomaly detection Feature:ml-results legacy - do not use v6.6.0 labels Nov 29, 2018
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui

Copy link
Contributor

@alvarezmelissa87 alvarezmelissa87 left a comment

Choose a reason for hiding this comment

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

LGTM ⚡️

@elasticmachine
Copy link
Contributor

💔 Build Failed

@timroes
Copy link
Contributor

timroes commented Nov 29, 2018

Jenkins, test this - ES master should accept the flag now

@elasticmachine
Copy link
Contributor

💔 Build Failed

Copy link
Contributor

@timroes timroes left a comment

Choose a reason for hiding this comment

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

Code LGTM, Can't judge if it's all necessary places

@timroes
Copy link
Contributor

timroes commented Nov 30, 2018

Jenkins, test this - change should now be on master snapshot

@elasticmachine
Copy link
Contributor

💔 Build Failed

Copy link
Contributor

@walterra walterra left a comment

Choose a reason for hiding this comment

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

LGTM

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Anomaly Detection ML anomaly detection Feature:ml-results legacy - do not use :ml non-issue Indicates to automation that a pull request should not appear in the release notes review v6.6.0 v7.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants