Skip to content

[ML] Force metric selection for Single Metric Viewer#48235

Merged
darnautov merged 17 commits intoelastic:masterfrom
darnautov:ML-18090-single-metric-force-partition-field
Oct 23, 2019
Merged

[ML] Force metric selection for Single Metric Viewer#48235
darnautov merged 17 commits intoelastic:masterfrom
darnautov:ML-18090-single-metric-force-partition-field

Conversation

@darnautov
Copy link
Copy Markdown
Contributor

@darnautov darnautov commented Oct 15, 2019 •
edited
Loading

Summary

Resolves #18090. Force metric selection for a Single Metric Viewer.

Oct-16-2019 14-10-13

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

@darnautov darnautov requested a review from a team as a code owner October 15, 2019 14:38
@darnautov darnautov self-assigned this Oct 15, 2019
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Oct 15, 2019

Pinging @elastic/ml-ui (:ml)

@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Oct 15, 2019

💚 Build Succeeded

alvarezmelissa87
alvarezmelissa87 approved these changes Oct 15, 2019
View reviewed changes
Copy link
Copy Markdown
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 overall ⚡️ Just a comment on having a message in future explaining when users see the blank page - that for single metric viewer it’s required to provide a single metric

walterra
walterra approved these changes Oct 15, 2019
View reviewed changes
Copy link
Copy Markdown
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
Copy Markdown
Contributor

elasticmachine commented Oct 15, 2019

💚 Build Succeeded

@sophiec20
Copy link
Copy Markdown
Contributor

sophiec20 commented Oct 16, 2019

What is the user experience for someone landing on this page, when a partitoning field has not been selected? Agree with @alvarezmelissa87 comment that we need to be helpful in this case.

@darnautov darnautov requested a review from a team as a code owner October 16, 2019 11:57
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Oct 16, 2019

💚 Build Succeeded

@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Oct 16, 2019

💚 Build Succeeded

@darnautov
Merge remote-tracking branch 'upstream/master' into ML-18090-single-m…
64be22a 
…etric-force-partition-field

# Conflicts:
#	x-pack/legacy/plugins/ml/public/timeseriesexplorer/timeseriesexplorer.js
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Oct 17, 2019

💚 Build Succeeded

@peteharverson
Copy link
Copy Markdown
Contributor

peteharverson commented Oct 17, 2019

I'm still able to reproduce the issue where the chart and table shows anomalies from all partitions:

image

@darnautov darnautov requested a review from a team as a code owner October 22, 2019 16:02
@darnautov darnautov requested a review from a team October 22, 2019 16:02
@darnautov darnautov requested a review from a team as a code owner October 22, 2019 16:02
@darnautov darnautov removed request for a team October 22, 2019 16:09
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Oct 22, 2019

💚 Build Succeeded

@alvarezmelissa87
Copy link
Copy Markdown
Contributor

alvarezmelissa87 commented Oct 22, 2019

Tested this locally - latest changes LGTM 👍

@darnautov
Copy link
Copy Markdown
Contributor Author

darnautov commented Oct 23, 2019

retest

@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Oct 23, 2019

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Oct 23, 2019

💔 Build Failed

peteharverson
peteharverson approved these changes Oct 23, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

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

Tested after latest commits, and LGTM

@darnautov
Copy link
Copy Markdown
Contributor Author

darnautov commented Oct 23, 2019

retest

ryankeairns
ryankeairns approved these changes Oct 23, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@ryankeairns ryankeairns left a comment

Choose a reason for hiding this comment

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

The SCSS change LGTM.

@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Oct 23, 2019

💚 Build Succeeded

@darnautov darnautov merged commit 206ab22 into elastic:master Oct 23, 2019
@darnautov darnautov deleted the ML-18090-single-metric-force-partition-field branch October 23, 2019 14:11
@darnautov darnautov mentioned this pull request Oct 23, 2019
Merged
darnautov added a commit to darnautov/kibana that referenced this pull request Oct 23, 2019
@darnautov
[ML] Force metric selection for Single Metric Viewer (elastic#48235)
74c9c48 
* [ML] force metric selection

* [ML] disable forecast if no metric selected

* [ML] check all controls

* [ML] fix for no partitioning fields

* [ML] warning message condition

* [ML] force metric selection

* [ML] disable forecast if no metric selected

* [ML] check all controls

* [ML] fix for no partitioning fields

* [ML] warning message condition

* [ML] fix criteria fields

* [ML] remove condition rendering for the tooltip

* [ML] fix tooltip location
darnautov added a commit that referenced this pull request Oct 24, 2019
@darnautov
[ML] Force metric selection for Single Metric Viewer (#48235) (#49046)
086c46a 
* [ML] force metric selection

* [ML] disable forecast if no metric selected

* [ML] check all controls

* [ML] fix for no partitioning fields

* [ML] warning message condition

* [ML] force metric selection

* [ML] disable forecast if no metric selected

* [ML] check all controls

* [ML] fix for no partitioning fields

* [ML] warning message condition

* [ML] fix criteria fields

* [ML] remove condition rendering for the tooltip

* [ML] fix tooltip location
@walterra walterra mentioned this pull request Nov 12, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@walterra walterra walterra approved these changes

@ryankeairns ryankeairns ryankeairns approved these changes

@alvarezmelissa87 alvarezmelissa87 alvarezmelissa87 approved these changes

@peteharverson peteharverson peteharverson approved these changes

@jgowdyelastic jgowdyelastic jgowdyelastic approved these changes

Assignees

@darnautov darnautov

Labels

Feature:Anomaly Detection ML anomaly detection :ml release_note:enhancement v7.6.0 v8.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[ML] Single Metric Viewer should require the end-user to select a partitioning field

8 participants

@darnautov @elasticmachine @sophiec20 @peteharverson @alvarezmelissa87 @walterra @ryankeairns @jgowdyelastic