-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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] MultiMetric/Population Job creation: Allow model plot enablement via checkbox #24914
Changes from 7 commits
92b9fbb
fd5debb
c41b081
610cff9
a2fac52
e15558d
f5d3ce6
e50e4e1
0d64859
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,153 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License; | ||
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
|
||
import React from 'react'; | ||
import ReactDOM from 'react-dom'; | ||
|
||
import { EnableModelPlotCheckbox } from './enable_model_plot_checkbox_view.js'; | ||
import { ml } from '../../../../../services/ml_api_service'; | ||
|
||
import { uiModules } from 'ui/modules'; | ||
const module = uiModules.get('apps/ml'); | ||
|
||
module.directive('mlEnableModelPlotCheckbox', function () { | ||
return { | ||
restrict: 'AE', | ||
replace: false, | ||
scope: { | ||
formConfig: '=', | ||
ui: '=ui', | ||
getJobFromConfig: '=' | ||
}, | ||
link: function ($scope, $element) { | ||
const STATUS = { | ||
FAILED: -1, | ||
NOT_RUNNING: 0, | ||
RUNNING: 1, | ||
FINISHED: 2, | ||
WARNING: 3, | ||
}; | ||
|
||
function errorHandler(error) { | ||
console.log('Cardinality could not be validated', error); | ||
$scope.ui.cardinalityValidator.status = STATUS.FAILED; | ||
$scope.ui.cardinalityValidator.message = 'Cardinality could not be validated'; | ||
} | ||
|
||
// Only model plot cardinality relevant | ||
// format:[{id:"cardinality_model_plot_high",modelPlotCardinality:11405}, {id:"cardinality_partition_field",fieldName:"clientip"}] | ||
function checkCardinalitySuccess(data) { | ||
const response = { | ||
success: true, | ||
}; | ||
// There were no fields to run cardinality on. | ||
if (Array.isArray(data) && data.length === 0) { | ||
return response; | ||
} | ||
|
||
for (let i = 0; i < data.length; i++) { | ||
if (data[i].id === 'success_cardinality') { | ||
break; | ||
} | ||
|
||
if (data[i].id === 'cardinality_model_plot_high') { | ||
response.success = false; | ||
response.highCardinality = data[i].modelPlotCardinality; | ||
break; | ||
} | ||
} | ||
|
||
return response; | ||
} | ||
|
||
function validateCardinality() { | ||
$scope.ui.cardinalityValidator.status = STATUS.RUNNING; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this isn't triggering the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hey I'm not quite sure what you mean it isn't triggering the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll try to reproduce on a local checkout. IMHO if we need to use |
||
$scope.ui.cardinalityValidator.message = ''; | ||
|
||
// create temporary job since cardinality validation expects that format | ||
const tempJob = $scope.getJobFromConfig($scope.formConfig); | ||
|
||
ml.validateCardinality(tempJob) | ||
.then((response) => { | ||
const validationResult = checkCardinalitySuccess(response); | ||
|
||
if (validationResult.success === true) { | ||
$scope.formConfig.enableModelPlot = true; | ||
$scope.ui.cardinalityValidator.status = STATUS.FINISHED; | ||
} else { | ||
$scope.ui.cardinalityValidator.message = `Creating model plots is resource intensive and not recommended | ||
where cardinality of the selected fields is greater than 100. Estimated cardinality | ||
for this job is ${validationResult.highCardinality}. | ||
If you enable model plot with this configuration we recommend you use a dedicated results index.`; | ||
|
||
$scope.ui.cardinalityValidator.status = STATUS.WARNING; | ||
// Go ahead and check the dedicated index box for them | ||
$scope.formConfig.useDedicatedIndex = true; | ||
// show the advanced section so the warning message is visible since validation failed | ||
$scope.ui.showAdvanced = true; | ||
} | ||
}) | ||
.catch(errorHandler); | ||
} | ||
|
||
// Re-validate cardinality for updated fields/splitField | ||
// when enable model plot is checked and form valid | ||
function revalidateCardinalityOnFieldChange() { | ||
if ($scope.formConfig.enableModelPlot === true && $scope.ui.formValid === true) { | ||
validateCardinality(); | ||
} | ||
} | ||
|
||
$scope.handleCheckboxChange = (isChecked) => { | ||
if (isChecked) { | ||
$scope.formConfig.enableModelPlot = true; | ||
validateCardinality(); | ||
} else { | ||
$scope.formConfig.enableModelPlot = false; | ||
$scope.ui.cardinalityValidator.status = STATUS.FINISHED; | ||
$scope.ui.cardinalityValidator.message = ''; | ||
} | ||
}; | ||
|
||
// Update checkbox on these changes | ||
$scope.$watch('ui.formValid', updateCheckbox, true); | ||
$scope.$watch('ui.cardinalityValidator.status', updateCheckbox, true); | ||
// MultiMetric: Fire off cardinality validatation when fields and/or split by field is updated | ||
$scope.$watch('formConfig.fields', revalidateCardinalityOnFieldChange, true); | ||
$scope.$watch('formConfig.splitField', revalidateCardinalityOnFieldChange, true); | ||
// Population: Fire off cardinality validatation when overField is updated | ||
$scope.$watch('formConfig.overField', revalidateCardinalityOnFieldChange, true); | ||
|
||
function updateCheckbox() { | ||
// disable if (check is running && checkbox checked) or (form is invalid && checkbox unchecked) | ||
const checkboxDisabled = ( | ||
($scope.ui.cardinalityValidator.status === STATUS.RUNNING && | ||
$scope.formConfig.enableModelPlot === true) || | ||
($scope.ui.formValid !== true && | ||
$scope.formConfig.enableModelPlot === false) | ||
); | ||
const validatorRunning = ($scope.ui.cardinalityValidator.status === STATUS.RUNNING); | ||
const warningStatus = ($scope.ui.cardinalityValidator.status === STATUS.WARNING); | ||
const checkboxText = (validatorRunning) ? 'Validating cardinality...' : 'Enable model plot'; | ||
|
||
const props = { | ||
checkboxDisabled, | ||
checkboxText, | ||
onCheckboxChange: $scope.handleCheckboxChange, | ||
warningContent: $scope.ui.cardinalityValidator.message, | ||
warningStatus, | ||
}; | ||
|
||
ReactDOM.render( | ||
React.createElement(EnableModelPlotCheckbox, props), | ||
$element[0] | ||
); | ||
} | ||
|
||
updateCheckbox(); | ||
} | ||
}; | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,98 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License; | ||
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
|
||
|
||
|
||
import PropTypes from 'prop-types'; | ||
import React, { Fragment, Component } from 'react'; | ||
|
||
import { | ||
EuiCallOut, | ||
EuiCheckbox, | ||
EuiFlexGroup, | ||
EuiFlexItem, | ||
EuiIconTip, | ||
} from '@elastic/eui'; | ||
|
||
|
||
export class EnableModelPlotCheckbox extends Component { | ||
constructor(props) { | ||
super(props); | ||
|
||
this.state = { | ||
checked: false, | ||
}; | ||
} | ||
|
||
tooltipContent = `Select to enable model plot. Stores model information along with results. | ||
Can add considerable overhead to the performance of the system.`; | ||
|
||
warningTitle = 'Proceed with caution!'; | ||
|
||
onChange = (e) => { | ||
this.setState({ | ||
checked: e.target.checked, | ||
}); | ||
|
||
this.props.onCheckboxChange(e.target.checked); | ||
}; | ||
|
||
renderWarningCallout = () => ( | ||
<Fragment> | ||
<EuiFlexGroup direction="column"> | ||
<EuiFlexItem grow={false}> | ||
<EuiCallOut | ||
title={this.warningTitle} | ||
color="warning" | ||
iconType="help" | ||
> | ||
<p> | ||
{this.props.warningContent} | ||
</p> | ||
</EuiCallOut> | ||
</EuiFlexItem> | ||
</EuiFlexGroup> | ||
</Fragment> | ||
); | ||
|
||
render() { | ||
return ( | ||
<Fragment> | ||
<EuiFlexGroup alignItems="center" gutterSize="s" responsive={false}> | ||
<EuiFlexItem grow={false}> | ||
<EuiCheckbox | ||
id="new_job_enable_model_plot" | ||
label={this.props.checkboxText} | ||
onChange={this.onChange} | ||
disabled={this.props.checkboxDisabled} | ||
checked={this.state.checked} | ||
/> | ||
</EuiFlexItem> | ||
|
||
<EuiFlexItem grow={false}> | ||
<EuiIconTip | ||
content={this.tooltipContent} | ||
size={'s'} | ||
/> | ||
</EuiFlexItem> | ||
</EuiFlexGroup> | ||
{ this.props.warningStatus && this.renderWarningCallout() } | ||
</Fragment> | ||
); | ||
} | ||
} | ||
|
||
EnableModelPlotCheckbox.propTypes = { | ||
checkboxDisabled: PropTypes.bool, | ||
checkboxText: PropTypes.string.isRequired, | ||
onCheckboxChange: PropTypes.func.isRequired, | ||
warningStatus: PropTypes.bool.isRequired, | ||
warningContent: PropTypes.string.isRequired, | ||
}; | ||
|
||
EnableModelPlotCheckbox.defaultProps = { | ||
checkboxDisabled: false, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License; | ||
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
|
||
|
||
import './enable_model_plot_checkbox_directive.js'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,3 +7,4 @@ | |
|
||
|
||
export { validateJob } from './job_validation'; | ||
export { validateCardinality } from './validate_cardinality'; |
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 would be nice if these IDs used for job validation messages were in a constants file, but not needed for this PR.
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.
Sounds good for the follow up PR with the advanced wizard. 👍