[ML] Validate manual model memory input #59056
Conversation
darnautov
added
release_note:enhancement
:ml
v8.0.0
Feature:Data Frame Analytics
darnautov
requested review from
alvarezmelissa87,
peteharverson and
jgowdyelastic
|
Pinging @elastic/ml-ui (:ml) |
| @@ -222,6 +261,32 @@ export const validateAdvancedEditor = (state: State): State => { | |||
| return state; | |||
| }; | |||
|
|
|||
| function validateMinMML(estimatedMml: string) { | |||
| return (mml: string) => { | |||
| // @ts-ignore | |||
There was a problem hiding this comment.
What are these ts-ignores for?
There was a problem hiding this comment.
there is a typing issue with @elastic/numeral', it complains about non-existing value
|
Code looks good overall! 😄Just a couple of comments. Once the mml has been automatically calculated if a lower number is manually input the
Adding space at the end of the manual input causes the regex check for validity to fail. Maybe just trim whitespace from the input string before doing the check?
|
|
@alvarezmelissa87 thanks for the review Melissa, good catch with memory input validation! I removed |
| if (errorKey === 'min') { | ||
| acc.push( | ||
| i18n.translate('xpack.ml.dataframe.analytics.create.modelMemoryUnitsMinError', { | ||
| defaultMessage: 'Model memory limit can not be lower than {mml}', |
There was a problem hiding this comment.
Nit, would use cannot here.
| export function validateMinMML(estimatedMml: string) { | ||
| return (mml: string) => { | ||
| // @ts-ignore | ||
| const mmlInBytes = numeral(mml.toUpperCase()).value(); |
There was a problem hiding this comment.
As soon as I set the job type in the flyout, the page goes blank with the following errors in the console:
darnautov
force-pushed
the
ML-58607-mml-validation
branch
from
10b266f
to
b42b65c
Compare
|
Looks good overall, @darnautov 😄 The only issue I can see is when |
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
* [ML] validate mml based on estimated value * [ML] better memoize * [ML] memoryInputValidator unit tests * [ML] cache mml errors * [ML] prevent override * [ML] fix validators, add unit tests * [ML] ignore typing issue with numeral * [ML] fix validateMinMML * [ML] fix useCreateAnalyticsForm test * [ML] setEstimatedModelMemoryLimit to the fallback value in case of an error
* [ML] validate mml based on estimated value * [ML] better memoize * [ML] memoryInputValidator unit tests * [ML] cache mml errors * [ML] prevent override * [ML] fix validators, add unit tests * [ML] ignore typing issue with numeral * [ML] fix validateMinMML * [ML] fix useCreateAnalyticsForm test * [ML] setEstimatedModelMemoryLimit to the fallback value in case of an error
Summary
Resolves #58607. Prevents MML user's manual input override by the result from
_explainAPI and adds extra validator for MML input.Checklist