Skip to content

Commit

Permalink
[ML] Adds docs_per_second to transform edit form. (#65365) (#65442)
Browse files Browse the repository at this point in the history 
Adds an option to edit the docs_per_second option for throttling for transforms.
  • Loading branch information
@walterra
walterra authored May 6, 2020
1 parent 6cb5fb6 commit 2c6c141
Show file tree
Hide file tree
Showing 4 changed files with 118 additions and 32 deletions.
4 changes: 4 additions & 0 deletions x-pack/plugins/transform/public/app/common/transform.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ export interface CreateRequestBody extends PreviewRequestBody {
index: IndexName;
};
frequency?: string;
settings?: {
max_page_search_size?: number;
docs_per_second?: number;
};
sync?: {
time: {
field: string;
Expand Down
19 changes: 13 additions & 6 deletions ...ions/transform_management/components/edit_transform_flyout/edit_transform_flyout_form.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,21 @@ export const EditTransformFlyoutForm: FC<EditTransformFlyoutFormProps> = ({
onChange={value => dispatch({ field: 'description', value })}
value={formFields.description.value}
/>
{/*
<EditTransformFlyoutFormTextInput
defaultValue={config.dest.index}
label={i18n.translate('xpack.transform.transformList.editFlyoutFormDestinationIndexLabel', {
defaultMessage: 'Destination Index',
errorMessages={formFields.docsPerSecond.errorMessages}
helpText={i18n.translate(
'xpack.transform.transformList.editFlyoutFormDocsPerSecondHelptext',
{
defaultMessage:
'To enable throttling, set a limit of documents per second of input documents.',
}
)}
label={i18n.translate('xpack.transform.transformList.editFlyoutFormdocsPerSecondLabel', {
defaultMessage: 'Documents per second',
})}
onChange={onChangeDestinationIndexHandler}
/>*/}
onChange={value => dispatch({ field: 'docsPerSecond', value })}
value={formFields.docsPerSecond.value}
/>
<EditTransformFlyoutFormTextInput
errorMessages={formFields.frequency.errorMessages}
helpText={i18n.translate('xpack.transform.transformList.editFlyoutFormFrequencyHelptext', {
Expand Down
45 changes: 40 additions & 5 deletions ...s/transform_management/components/edit_transform_flyout/use_edit_transform_flyout.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import {
formReducerFactory,
frequencyValidator,
getDefaultState,
numberAboveZeroValidator,
FormField,
} from './use_edit_transform_flyout';

const getTransformConfigMock = (): TransformPivotConfig => ({
Expand Down Expand Up @@ -43,14 +45,21 @@ const getTransformConfigMock = (): TransformPivotConfig => ({
description: 'the-description',
});

const getDescriptionFieldMock = (value = '') => ({
const getDescriptionFieldMock = (value = ''): FormField => ({
isOptional: true,
value,
errorMessages: [],
validator: 'string',
});

const getFrequencyFieldMock = (value = '') => ({
const getDocsPerSecondFieldMock = (value = ''): FormField => ({
isOptional: true,
value,
errorMessages: [],
validator: 'numberAboveZero',
});

const getFrequencyFieldMock = (value = ''): FormField => ({
isOptional: true,
value,
errorMessages: [],
Expand All @@ -63,13 +72,16 @@ describe('Transform: applyFormFieldsToTransformConfig()', () => {

const updateConfig = applyFormFieldsToTransformConfig(transformConfigMock, {
description: getDescriptionFieldMock(transformConfigMock.description),
docsPerSecond: getDocsPerSecondFieldMock(),
frequency: getFrequencyFieldMock(),
});

// This case will return an empty object. In the actual UI, this case should not happen
// because the Update-Button will be disabled when no form field was changed.
expect(Object.keys(updateConfig)).toHaveLength(0);
expect(updateConfig.description).toBe(undefined);
// `docs_per_second` is nested under `settings` so we're just checking against that.
expect(updateConfig.settings).toBe(undefined);
expect(updateConfig.frequency).toBe(undefined);
});

Expand All @@ -80,23 +92,28 @@ describe('Transform: applyFormFieldsToTransformConfig()', () => {

const updateConfig = applyFormFieldsToTransformConfig(transformConfigMock, {
description: getDescriptionFieldMock('the-new-description'),
frequency: getFrequencyFieldMock(undefined),
docsPerSecond: getDocsPerSecondFieldMock('10'),
frequency: getFrequencyFieldMock('1m'),
});

expect(Object.keys(updateConfig)).toHaveLength(1);
expect(Object.keys(updateConfig)).toHaveLength(3);
expect(updateConfig.description).toBe('the-new-description');
expect(updateConfig.frequency).toBe(undefined);
expect(updateConfig.settings?.docs_per_second).toBe(10);
expect(updateConfig.frequency).toBe('1m');
});

test('should only include changed form fields', () => {
const transformConfigMock = getTransformConfigMock();
const updateConfig = applyFormFieldsToTransformConfig(transformConfigMock, {
description: getDescriptionFieldMock('the-updated-description'),
docsPerSecond: getDocsPerSecondFieldMock(),
frequency: getFrequencyFieldMock(),
});

expect(Object.keys(updateConfig)).toHaveLength(1);
expect(updateConfig.description).toBe('the-updated-description');
// `docs_per_second` is nested under `settings` so we're just checking against that.
expect(updateConfig.settings).toBe(undefined);
expect(updateConfig.frequency).toBe(undefined);
});
});
Expand Down Expand Up @@ -166,3 +183,21 @@ describe('Transform: frequencyValidator()', () => {
expect(frequencyValidator('59m')).toHaveLength(0);
});
});

describe('Transform: numberValidator()', () => {
test('it should only allow numbers', () => {
// numberValidator() returns an array of error messages so
// an array with a length of 0 means a successful validation.

// invalid
expect(numberAboveZeroValidator('a-string')).toHaveLength(1);
expect(numberAboveZeroValidator('0s')).toHaveLength(1);
expect(numberAboveZeroValidator('1m')).toHaveLength(1);
expect(numberAboveZeroValidator(-1)).toHaveLength(1);
expect(numberAboveZeroValidator(0)).toHaveLength(1);

// valid
expect(numberAboveZeroValidator(1)).toHaveLength(0);
expect(numberAboveZeroValidator('1')).toHaveLength(0);
});
});
82 changes: 61 additions & 21 deletions ...ctions/transform_management/components/edit_transform_flyout/use_edit_transform_flyout.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,29 +11,43 @@ import { i18n } from '@kbn/i18n';

import { TransformPivotConfig } from '../../../../common';

// A Validator function takes in a value to check and returns an array of error messages.
// If no messages (empty array) get returned, the value is valid.
type Validator = (arg: any) => string[];

// Note on the form validation and input components used:
// All inputs use `EuiFieldText` which means all form values will be treated as strings.
// This means we cast other formats like numbers coming from the transform config to strings,
// then revalidate them and cast them again to number before submitting a transform update.
// We do this so we have fine grained control over field validation and the option to
// cast to special values like `null` for disabling `docs_per_second`.
const numberAboveZeroNotValidErrorMessage = i18n.translate(
'xpack.transform.transformList.editFlyoutFormNumberNotValidErrorMessage',
{
defaultMessage: 'Value needs to be a number above zero.',
}
);
export const numberAboveZeroValidator: Validator = arg =>
!isNaN(arg) && parseInt(arg, 10) > 0 ? [] : [numberAboveZeroNotValidErrorMessage];

// The way the current form is set up, this validator is just a sanity check,
// it should never trigger an error, because `EuiFieldText` always returns a string.
const stringNotValidErrorMessage = i18n.translate(
'xpack.transform.transformList.editFlyoutFormStringNotValidErrorMessage',
{
defaultMessage: 'Value needs to be of type string.',
}
);

type Validator = (arg: any) => string[];

// The way the current form is set up,
// this validator is just a sanity check,
// it should never trigger an error.
const stringValidator: Validator = arg =>
typeof arg === 'string' ? [] : [stringNotValidErrorMessage];

// Only allow frequencies in the form of 1s/1h etc.
const frequencyNotValidErrorMessage = i18n.translate(
'xpack.transform.transformList.editFlyoutFormFrequencyNotValidErrorMessage',
{
defaultMessage: 'The frequency value is not valid.',
}
);

// Only allow frequencies in the form of 1s/1h etc.
export const frequencyValidator: Validator = arg => {
if (typeof arg !== 'string' || arg === null) {
return [stringNotValidErrorMessage];
Expand All @@ -58,33 +72,37 @@ export const frequencyValidator: Validator = arg => {
);
};

interface Validate {
[key: string]: Validator;
}
type Validators = 'string' | 'frequency' | 'numberAboveZero';

type Validate = {
[key in Validators]: Validator;
};

const validate: Validate = {
string: stringValidator,
frequency: frequencyValidator,
numberAboveZero: numberAboveZeroValidator,
};

interface Field {
export interface FormField {
errorMessages: string[];
isOptional: boolean;
validator: keyof typeof validate;
validator: keyof Validate;
value: string;
}

const defaultField: Field = {
const defaultField: FormField = {
errorMessages: [],
isOptional: true,
validator: 'string',
value: '',
};

interface EditTransformFlyoutFieldsState {
[key: string]: Field;
description: Field;
frequency: Field;
[key: string]: FormField;
description: FormField;
frequency: FormField;
docsPerSecond: FormField;
}

export interface EditTransformFlyoutState {
Expand All @@ -101,18 +119,22 @@ interface Action {
}

// Some attributes can have a value of `null` to trigger
// a reset to the default value.
// a reset to the default value, or in the case of `docs_per_second`
// `null` is used to disable throttling.
interface UpdateTransformPivotConfig {
description: string;
frequency: string;
settings: {
docs_per_second: number | null;
};
}

// Takes in the form configuration and returns a
// request object suitable to be sent to the
// transform update API endpoint.
export const applyFormFieldsToTransformConfig = (
config: TransformPivotConfig,
{ description, frequency }: EditTransformFlyoutFieldsState
{ description, docsPerSecond, frequency }: EditTransformFlyoutFieldsState
): Partial<UpdateTransformPivotConfig> => {
const updateConfig: Partial<UpdateTransformPivotConfig> = {};

Expand All @@ -132,6 +154,16 @@ export const applyFormFieldsToTransformConfig = (
updateConfig.description = description.value;
}

// if the input field was left empty,
// fall back to the default value of `null`
// which will disable throttling.
const docsPerSecondFormValue =
docsPerSecond.value !== '' ? parseInt(docsPerSecond.value, 10) : null;
const docsPerSecondConfigValue = config.settings?.docs_per_second ?? null;
if (docsPerSecondFormValue !== docsPerSecondConfigValue) {
updateConfig.settings = { docs_per_second: docsPerSecondFormValue };
}

return updateConfig;
};

Expand All @@ -141,6 +173,11 @@ export const getDefaultState = (config: TransformPivotConfig): EditTransformFlyo
formFields: {
description: { ...defaultField, value: config?.description ?? '' },
frequency: { ...defaultField, value: config?.frequency ?? '', validator: 'frequency' },
docsPerSecond: {
...defaultField,
value: config?.settings?.docs_per_second?.toString() ?? '',
validator: 'numberAboveZero',
},
},
isFormTouched: false,
isFormValid: true,
Expand All @@ -154,10 +191,13 @@ const isFormValid = (fieldsState: EditTransformFlyoutFieldsState) =>
// Updates a form field with its new value,
// runs validation and populates
// `errorMessages` if any errors occur.
const formFieldReducer = (state: Field, value: string): Field => {
const formFieldReducer = (state: FormField, value: string): FormField => {
return {
...state,
errorMessages: state.isOptional && value.length === 0 ? [] : validate[state.validator](value),
errorMessages:
state.isOptional && typeof value === 'string' && value.length === 0
? []
: validate[state.validator](value),
value,
};
};
Expand Down

0 comments on commit 2c6c141

Please sign in to comment.