Skip to content

Commit

Permalink
[ML] fix mutation of the original job during the cloning
Browse files Browse the repository at this point in the history
  • Loading branch information
darnautov committed Mar 17, 2020
1 parent 6a97e1b commit 36485ef
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 24 deletions.
12 changes: 12 additions & 0 deletions x-pack/plugins/ml/common/types/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,15 @@ export function dictionaryToArray<TValue>(dict: Dictionary<TValue>): TValue[] {
export type DeepPartial<T> = {
[P in keyof T]?: DeepPartial<T[P]>;
};

export type DeepReadonly<T> = T extends Array<infer R>
? ReadonlyArray<DeepReadonly<T>>
: T extends Function
? T
: T extends object
? DeepReadonlyObject<T>
: T;

type DeepReadonlyObject<T> = {
readonly [P in keyof T]: DeepReadonly<T[P]>;
};
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@

import { EuiButtonEmpty } from '@elastic/eui';
import React, { FC } from 'react';
import { isEqual } from 'lodash';
import { isEqual, cloneDeep } from 'lodash';
import { i18n } from '@kbn/i18n';
import { DeepReadonly } from '../../../../../../../common/types/common';
import { DataFrameAnalyticsConfig, isOutlierAnalysis } from '../../../../common';
import { isClassificationAnalysis, isRegressionAnalysis } from '../../../../common/analytics';
import { CreateAnalyticsFormProps } from '../../hooks/use_create_analytics_form';
Expand Down Expand Up @@ -261,20 +262,25 @@ export type CloneDataFrameAnalyticsConfig = Omit<
'id' | 'version' | 'create_time'
>;

export function extractCloningConfig(
originalConfig: DataFrameAnalyticsConfig
): CloneDataFrameAnalyticsConfig {
const {
// Omit non-relevant props from the configuration
id,
version,
create_time,
...cloneConfig
} = originalConfig;

// Reset the destination index
cloneConfig.dest.index = '';
return cloneConfig;
/**
* Gets complete original configuration as an input
* and returns the config for cloning omitting
* non-relevant parameters and resetting the destination index.
*/
export function extractCloningConfig({
id,
version,
create_time,
...configToClone
}: DeepReadonly<DataFrameAnalyticsConfig>): CloneDataFrameAnalyticsConfig {
return (cloneDeep({
...configToClone,
dest: {
...configToClone.dest,
// Reset the destination index
index: '',
},
}) as unknown) as CloneDataFrameAnalyticsConfig;
}

export function getCloneAction(createAnalyticsForm: CreateAnalyticsFormProps) {
Expand All @@ -284,7 +290,7 @@ export function getCloneAction(createAnalyticsForm: CreateAnalyticsFormProps) {

const { actions } = createAnalyticsForm;

const onClick = async (item: DataFrameAnalyticsListRow) => {
const onClick = async (item: DeepReadonly<DataFrameAnalyticsListRow>) => {
await actions.setJobClone(item.config);
};

Expand All @@ -298,7 +304,7 @@ export function getCloneAction(createAnalyticsForm: CreateAnalyticsFormProps) {
}

interface CloneActionProps {
item: DataFrameAnalyticsListRow;
item: DeepReadonly<DataFrameAnalyticsListRow>;
createAnalyticsForm: CreateAnalyticsFormProps;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import React from 'react';
import { i18n } from '@kbn/i18n';
import { EuiButtonEmpty, EuiToolTip } from '@elastic/eui';
import { DeepReadonly } from '../../../../../../../common/types/common';

import {
checkPermission,
Expand Down Expand Up @@ -107,7 +108,7 @@ export const getActions = (createAnalyticsForm: CreateAnalyticsFormProps) => {
},
},
{
render: (item: DataFrameAnalyticsListRow) => {
render: (item: DeepReadonly<DataFrameAnalyticsListRow>) => {
return <CloneAction item={item} createAnalyticsForm={createAnalyticsForm} />;
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { DeepReadonly } from '../../../../../../../common/types/common';
import { DataFrameAnalyticsConfig } from '../../../../common';
import { FormMessage, State, SourceIndexMap } from './state';

Expand Down Expand Up @@ -64,7 +65,7 @@ export type Action =
| { type: ACTION.SET_JOB_CONFIG; payload: State['jobConfig'] }
| { type: ACTION.SET_JOB_IDS; jobIds: State['jobIds'] }
| { type: ACTION.SET_ESTIMATED_MODEL_MEMORY_LIMIT; value: State['estimatedModelMemoryLimit'] }
| { type: ACTION.SET_JOB_CLONE; cloneJob: DataFrameAnalyticsConfig };
| { type: ACTION.SET_JOB_CLONE; cloneJob: DeepReadonly<DataFrameAnalyticsConfig> };

// Actions wrapping the dispatcher exposed by the custom hook
export interface ActionDispatchers {
Expand All @@ -79,5 +80,5 @@ export interface ActionDispatchers {
startAnalyticsJob: () => void;
switchToAdvancedEditor: () => void;
setEstimatedModelMemoryLimit: (value: State['estimatedModelMemoryLimit']) => void;
setJobClone: (cloneJob: DataFrameAnalyticsConfig) => Promise<void>;
setJobClone: (cloneJob: DeepReadonly<DataFrameAnalyticsConfig>) => Promise<void>;
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/

import { EuiComboBoxOptionOption } from '@elastic/eui';
import { DeepPartial } from '../../../../../../../common/types/common';
import { DeepPartial, DeepReadonly } from '../../../../../../../common/types/common';
import { checkPermission } from '../../../../../privilege/check_privilege';
import { mlNodesAvailable } from '../../../../../ml_nodes_check';

Expand Down Expand Up @@ -91,7 +91,7 @@ export interface State {
jobIds: DataFrameAnalyticsId[];
requestMessages: FormMessage[];
estimatedModelMemoryLimit: string;
cloneJob?: DataFrameAnalyticsConfig;
cloneJob?: DeepReadonly<DataFrameAnalyticsConfig>;
}

export const getInitialState = (): State => ({
Expand Down Expand Up @@ -195,7 +195,7 @@ export const getJobConfigFromFormState = (
* For cloning we keep job id and destination index empty.
*/
export function getCloneFormStateFromJobConfig(
analyticsJobConfig: CloneDataFrameAnalyticsConfig
analyticsJobConfig: Readonly<CloneDataFrameAnalyticsConfig>
): Partial<State['form']> {
const jobType = Object.keys(analyticsJobConfig.analysis)[0] as ANALYSIS_CONFIG_TYPE;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { useReducer } from 'react';
import { i18n } from '@kbn/i18n';

import { SimpleSavedObject } from 'kibana/public';
import { DeepReadonly } from '../../../../../../../common/types/common';
import { ml } from '../../../../../services/ml_api_service';
import { useMlContext } from '../../../../../contexts/ml';

Expand Down Expand Up @@ -308,7 +309,7 @@ export const useCreateAnalyticsForm = (): CreateAnalyticsFormProps => {
dispatch({ type: ACTION.SET_ESTIMATED_MODEL_MEMORY_LIMIT, value });
};

const setJobClone = async (cloneJob: DataFrameAnalyticsConfig) => {
const setJobClone = async (cloneJob: DeepReadonly<DataFrameAnalyticsConfig>) => {
resetForm();
await prepareFormValidation();

Expand Down

0 comments on commit 36485ef

Please sign in to comment.