Skip to content
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] Transforms: Add ability to delete dest index & index pattern when deleting transform job #67922

Merged
merged 19 commits into from
Jun 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
9a72929
[Transform] Add ability to delete dest index & index pattern when del…
qn895 Jun 2, 2020
a9620c6
[Transform] Combine success messages when deleting job transforms
qn895 Jun 2, 2020
f2d672b
[Transform] Change preview text length to 50
qn895 Jun 2, 2020
1e6bd0c
[Transform] Add API integration tests for delete_transforms
qn895 Jun 3, 2020
b32b884
[Transform] Update tests and fix i18n duplicates
qn895 Jun 4, 2020
90a2d5e
[Transform] Update bulk delete messages
qn895 Jun 4, 2020
d87248d
[Transform] Update tests
qn895 Jun 4, 2020
d3c37b5
[Transform] Rename transform job -> transform, fix ts
qn895 Jun 4, 2020
ec3acd3
[Transform] Add await to userCanDelete callback
qn895 Jun 4, 2020
1269b1d
[Transform] Remove .toLowerCase check in getIP
qn895 Jun 4, 2020
47ec2cc
Merge remote-tracking branch 'upstream/master' into transform-delete-…
qn895 Jun 4, 2020
d8f901c
[Transform] Update tests to include waitForIndicesToExist
qn895 Jun 5, 2020
cd293f2
[Transform] Refactor delete transform schema
qn895 Jun 5, 2020
8e7b21a
[Transform] Add spacer, rename indices,
qn895 Jun 5, 2020
def2ccf
[Transform] Refactor new IndexService
qn895 Jun 5, 2020
d340b35
Merge remote-tracking branch 'upstream/master' into transform-delete-…
qn895 Jun 9, 2020
f2b70fb
[Transform] Extend MLResponseError in extractErrorMessage
qn895 Jun 9, 2020
a5fda7f
[Transform] Add unit tests extractErrorMessage and modify type defs
qn895 Jun 9, 2020
6f0fc96
[Transform] Refactor extractErrorMessage to account for boom responses
qn895 Jun 9, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 78 additions & 0 deletions x-pack/plugins/ml/common/util/errors.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
/*
* 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 {
BoomResponse,
extractErrorMessage,
MLCustomHttpResponseOptions,
MLResponseError,
} from './errors';
import { ResponseError } from 'kibana/server';

describe('ML - error message utils', () => {
describe('extractErrorMessage', () => {
test('returns just the error message', () => {
const testMsg = 'Saved object [index-pattern/blahblahblah] not found';

const bodyWithNestedErrorMsg: MLCustomHttpResponseOptions<MLResponseError> = {
body: {
message: {
msg: testMsg,
},
},
statusCode: 404,
};
expect(extractErrorMessage(bodyWithNestedErrorMsg)).toBe(testMsg);

const bodyWithStringMsg: MLCustomHttpResponseOptions<MLResponseError> = {
body: {
msg: testMsg,
},
statusCode: 404,
};
expect(extractErrorMessage(bodyWithStringMsg)).toBe(testMsg);

const bodyWithStringMessage: MLCustomHttpResponseOptions<ResponseError> = {
body: {
message: testMsg,
},
statusCode: 404,
};
expect(extractErrorMessage(bodyWithStringMessage)).toBe(testMsg);

const bodyWithString: MLCustomHttpResponseOptions<ResponseError> = {
body: testMsg,
statusCode: 404,
};
expect(extractErrorMessage(bodyWithString)).toBe(testMsg);

const bodyWithError: MLCustomHttpResponseOptions<ResponseError> = {
body: new Error(testMsg),
statusCode: 404,
};
expect(extractErrorMessage(bodyWithError)).toBe(testMsg);

const bodyWithBoomError: MLCustomHttpResponseOptions<BoomResponse> = {
statusCode: 404,
body: {
data: [],
isBoom: true,
isServer: false,
output: {
statusCode: 404,
payload: {
statusCode: 404,
error: testMsg,
message: testMsg,
},
headers: {},
},
},
};
expect(extractErrorMessage(bodyWithBoomError)).toBe(testMsg);
});
});
});
75 changes: 75 additions & 0 deletions x-pack/plugins/ml/common/util/errors.ts
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 { ResponseError, ResponseHeaders } from 'kibana/server';
import { isErrorResponse } from '../types/errors';

export function getErrorMessage(error: any) {
Expand All @@ -17,3 +18,77 @@ export function getErrorMessage(error: any) {

return JSON.stringify(error);
}

// Adding temporary types until Kibana ResponseError is updated

export interface BoomResponse {
data: any;
isBoom: boolean;
isServer: boolean;
output: {
statusCode: number;
payload: {
statusCode: number;
error: string;
message: string;
};
headers: {};
};
}
export type MLResponseError =
| {
message: {
msg: string;
};
}
| { msg: string };

export interface MLCustomHttpResponseOptions<
T extends ResponseError | MLResponseError | BoomResponse
> {
/** HTTP message to send to the client */
body?: T;
/** HTTP Headers with additional information about response */
headers?: ResponseHeaders;
statusCode: number;
}

export const extractErrorMessage = (
Copy link
Contributor

Choose a reason for hiding this comment

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

this function contains plenty of if statements, worth to covert with unit tests and provide a better typing

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll do this an a follow up PR

error:
| MLCustomHttpResponseOptions<MLResponseError | ResponseError | BoomResponse>
| undefined
| string
): string => {
// extract only the error message within the response error coming from Kibana, Elasticsearch, and our own ML messages

if (typeof error === 'string') {
return error;
}
if (error?.body === undefined) return '';

if (typeof error.body === 'string') {
return error.body;
}
if (
typeof error.body === 'object' &&
'output' in error.body &&
error.body.output.payload.message
) {
return error.body.output.payload.message;
}

if (typeof error.body === 'object' && 'msg' in error.body && typeof error.body.msg === 'string') {
return error.body.msg;
}

if (typeof error.body === 'object' && 'message' in error.body) {
if (typeof error.body.message === 'string') {
return error.body.message;
}
if (!(error.body.message instanceof Error) && typeof (error.body.message.msg === 'string')) {
return error.body.message.msg;
}
}
// If all else fail return an empty message instead of JSON.stringify
return '';
};
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
} from '@elastic/eui';
import { IIndexPattern } from 'src/plugins/data/common';
import { FormattedMessage } from '@kbn/i18n/react';
import { extractErrorMessage } from '../../../../../../../common/util/errors';
import {
deleteAnalytics,
deleteAnalyticsAndDestIndex,
Expand All @@ -29,7 +30,6 @@ import {
} from '../../../../../capabilities/check_capabilities';
import { useMlKibana } from '../../../../../contexts/kibana';
import { isDataFrameAnalyticsRunning, DataFrameAnalyticsListRow } from './common';
import { extractErrorMessage } from '../../../../../util/error_utils';

interface DeleteActionProps {
item: DataFrameAnalyticsListRow;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,14 @@
* you may not use this file except in compliance with the Elastic License.
*/
import { i18n } from '@kbn/i18n';
import { extractErrorMessage } from '../../../../../../../common/util/errors';
import { getToastNotifications } from '../../../../../util/dependency_cache';
import { ml } from '../../../../../services/ml_api_service';
import { refreshAnalyticsList$, REFRESH_ANALYTICS_LIST_STATE } from '../../../../common';
import {
isDataFrameAnalyticsFailed,
DataFrameAnalyticsListRow,
} from '../../components/analytics_list/common';
import { extractErrorMessage } from '../../../../../util/error_utils';

export const deleteAnalytics = async (d: DataFrameAnalyticsListRow) => {
const toastNotifications = getToastNotifications();
Expand Down
32 changes: 0 additions & 32 deletions x-pack/plugins/ml/public/application/util/error_utils.ts

This file was deleted.

1 change: 1 addition & 0 deletions x-pack/plugins/ml/public/shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export * from '../common/types/audit_message';

export * from '../common/util/anomaly_utils';
export * from '../common/util/errors';

export * from '../common/util/validators';

export * from './application/formatters/metric_change_description';
Expand Down
17 changes: 17 additions & 0 deletions x-pack/plugins/transform/common/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,20 @@ export interface ResultData {
export interface TransformEndpointResult {
[key: string]: ResultData;
}

export interface DeleteTransformEndpointRequest {
transformsInfo: TransformEndpointRequest[];
deleteDestIndex?: boolean;
deleteDestIndexPattern?: boolean;
}

export interface DeleteTransformStatus {
transformDeleted: ResultData;
destIndexDeleted?: ResultData;
destIndexPatternDeleted?: ResultData;
destinationIndex?: string | undefined;
}

export interface DeleteTransformEndpointResult {
[key: string]: DeleteTransformStatus;
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,14 @@ const MAX_SIMPLE_MESSAGE_LENGTH = 140;
interface ToastNotificationTextProps {
overlays: CoreStart['overlays'];
text: any;
previewTextLength?: number;
}

export const ToastNotificationText: FC<ToastNotificationTextProps> = ({ overlays, text }) => {
export const ToastNotificationText: FC<ToastNotificationTextProps> = ({
overlays,
text,
previewTextLength,
}) => {
if (typeof text === 'string' && text.length <= MAX_SIMPLE_MESSAGE_LENGTH) {
return text;
}
Expand All @@ -46,8 +51,9 @@ export const ToastNotificationText: FC<ToastNotificationTextProps> = ({ overlays

const unformattedText = text.message ? text.message : text;
const formattedText = typeof unformattedText === 'object' ? JSON.stringify(text, null, 2) : text;
const previewText = `${formattedText.substring(0, 140)}${
formattedText.length > 140 ? ' ...' : ''
const textLength = previewTextLength ?? 140;
const previewText = `${formattedText.substring(0, textLength)}${
formattedText.length > textLength ? ' ...' : ''
}`;

const openModal = () => {
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/transform/public/app/hooks/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

export { useApi } from './use_api';
export { useGetTransforms } from './use_get_transforms';
export { useDeleteTransforms } from './use_delete_transform';
export { useDeleteTransforms, useDeleteIndexAndTargetIndex } from './use_delete_transform';
export { useStartTransforms } from './use_start_transform';
export { useStopTransforms } from './use_stop_transform';
export { useRequest } from './use_request';
15 changes: 11 additions & 4 deletions x-pack/plugins/transform/public/app/hooks/use_api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,12 @@
*/

import { useMemo } from 'react';
import { TransformEndpointRequest, TransformEndpointResult, TransformId } from '../../../common';
import {
TransformId,
TransformEndpointRequest,
TransformEndpointResult,
DeleteTransformEndpointResult,
} from '../../../common';
import { API_BASE_PATH } from '../../../common/constants';

import { useAppDependencies } from '../app_dependencies';
Expand Down Expand Up @@ -40,10 +45,12 @@ export const useApi = () => {
});
},
deleteTransforms(
transformsInfo: TransformEndpointRequest[]
): Promise<TransformEndpointResult> {
transformsInfo: TransformEndpointRequest[],
deleteDestIndex: boolean | undefined,
deleteDestIndexPattern: boolean | undefined
): Promise<DeleteTransformEndpointResult> {
return http.post(`${API_BASE_PATH}delete_transforms`, {
body: JSON.stringify(transformsInfo),
body: JSON.stringify({ transformsInfo, deleteDestIndex, deleteDestIndexPattern }),
});
},
getTransformsPreview(obj: PreviewRequestBody): Promise<GetTransformsResponse> {
Expand Down
Loading