From 518a96edd3c0f818e685a6f64bd9ece97bb5a7d5 Mon Sep 17 00:00:00 2001 From: Samiul Monir Date: Wed, 30 Oct 2024 12:54:35 -0400 Subject: [PATCH 1/5] Disabling try again button while model is still deploying --- .../trained_models_deployment_modal.test.tsx | 7 ++----- .../details_page/trained_models_deployment_modal.tsx | 5 ++++- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/x-pack/plugins/index_management/__jest__/client_integration/index_details_page/trained_models_deployment_modal.test.tsx b/x-pack/plugins/index_management/__jest__/client_integration/index_details_page/trained_models_deployment_modal.test.tsx index 0ba8ca201d40d..528a7f167f1c1 100644 --- a/x-pack/plugins/index_management/__jest__/client_integration/index_details_page/trained_models_deployment_modal.test.tsx +++ b/x-pack/plugins/index_management/__jest__/client_integration/index_details_page/trained_models_deployment_modal.test.tsx @@ -173,11 +173,8 @@ describe('When semantic_text is enabled', () => { ); }); - it('should call saveMappings if refresh button is pressed', async () => { - await act(async () => { - find('tryAgainModalButton').simulate('click'); - }); - expect(saveMappings.mock.calls).toHaveLength(1); + it('should disable the try again button', () => { + expect(find('tryAgainModalButton').props().disabled).toBe(true); }); it('should disable the force save mappings button if checkbox is not checked', async () => { expect(find('forceSaveMappingsButton').props().disabled).toBe(true); diff --git a/x-pack/plugins/index_management/public/application/sections/home/index_list/details_page/trained_models_deployment_modal.tsx b/x-pack/plugins/index_management/public/application/sections/home/index_list/details_page/trained_models_deployment_modal.tsx index b2e9a1339c3fb..5d8672ea0d24a 100644 --- a/x-pack/plugins/index_management/public/application/sections/home/index_list/details_page/trained_models_deployment_modal.tsx +++ b/x-pack/plugins/index_management/public/application/sections/home/index_list/details_page/trained_models_deployment_modal.tsx @@ -155,7 +155,9 @@ export function TrainedModelsDeploymentModal({ aria-labelledby={modalTitleId} onClose={closeModal} data-test-subj="trainedModelsDeploymentModal" - initialFocus="[data-test-subj=tryAgainModalButton]" + initialFocus={ + erroredDeployments.length === 0 ? undefined : '[data-test-subj=tryAgainModalButton]' + } > @@ -260,6 +262,7 @@ export function TrainedModelsDeploymentModal({ onClick={saveMappings} data-test-subj="tryAgainModalButton" isLoading={saveMappingsLoading} + disabled={erroredDeployments.length === 0} > {i18n.translate( 'xpack.idxMgmt.indexDetails.trainedModelsDeploymentModal.tryAgainButtonLabel', From 7ceb8739f658d6843637388df5837f6d0c533109 Mon Sep 17 00:00:00 2001 From: Samiul Monir Date: Thu, 31 Oct 2024 11:49:26 -0400 Subject: [PATCH 2/5] revert back the disable logic for try again button --- .../trained_models_deployment_modal.test.tsx | 7 +++++-- .../details_page/trained_models_deployment_modal.tsx | 5 +---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/x-pack/plugins/index_management/__jest__/client_integration/index_details_page/trained_models_deployment_modal.test.tsx b/x-pack/plugins/index_management/__jest__/client_integration/index_details_page/trained_models_deployment_modal.test.tsx index 528a7f167f1c1..0ba8ca201d40d 100644 --- a/x-pack/plugins/index_management/__jest__/client_integration/index_details_page/trained_models_deployment_modal.test.tsx +++ b/x-pack/plugins/index_management/__jest__/client_integration/index_details_page/trained_models_deployment_modal.test.tsx @@ -173,8 +173,11 @@ describe('When semantic_text is enabled', () => { ); }); - it('should disable the try again button', () => { - expect(find('tryAgainModalButton').props().disabled).toBe(true); + it('should call saveMappings if refresh button is pressed', async () => { + await act(async () => { + find('tryAgainModalButton').simulate('click'); + }); + expect(saveMappings.mock.calls).toHaveLength(1); }); it('should disable the force save mappings button if checkbox is not checked', async () => { expect(find('forceSaveMappingsButton').props().disabled).toBe(true); diff --git a/x-pack/plugins/index_management/public/application/sections/home/index_list/details_page/trained_models_deployment_modal.tsx b/x-pack/plugins/index_management/public/application/sections/home/index_list/details_page/trained_models_deployment_modal.tsx index 5d8672ea0d24a..b2e9a1339c3fb 100644 --- a/x-pack/plugins/index_management/public/application/sections/home/index_list/details_page/trained_models_deployment_modal.tsx +++ b/x-pack/plugins/index_management/public/application/sections/home/index_list/details_page/trained_models_deployment_modal.tsx @@ -155,9 +155,7 @@ export function TrainedModelsDeploymentModal({ aria-labelledby={modalTitleId} onClose={closeModal} data-test-subj="trainedModelsDeploymentModal" - initialFocus={ - erroredDeployments.length === 0 ? undefined : '[data-test-subj=tryAgainModalButton]' - } + initialFocus="[data-test-subj=tryAgainModalButton]" > @@ -262,7 +260,6 @@ export function TrainedModelsDeploymentModal({ onClick={saveMappings} data-test-subj="tryAgainModalButton" isLoading={saveMappingsLoading} - disabled={erroredDeployments.length === 0} > {i18n.translate( 'xpack.idxMgmt.indexDetails.trainedModelsDeploymentModal.tryAgainButtonLabel', From df1051a23d5ce310b5f693e3f22a5fa8269732b4 Mon Sep 17 00:00:00 2001 From: Samiul Monir Date: Thu, 31 Oct 2024 18:16:31 -0400 Subject: [PATCH 3/5] Adding restriction to avoid redeploying models from kibana --- .../index_list/details_page/trained_models_deployment_modal.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/index_management/public/application/sections/home/index_list/details_page/trained_models_deployment_modal.tsx b/x-pack/plugins/index_management/public/application/sections/home/index_list/details_page/trained_models_deployment_modal.tsx index b2e9a1339c3fb..4b41bcf139d88 100644 --- a/x-pack/plugins/index_management/public/application/sections/home/index_list/details_page/trained_models_deployment_modal.tsx +++ b/x-pack/plugins/index_management/public/application/sections/home/index_list/details_page/trained_models_deployment_modal.tsx @@ -136,7 +136,7 @@ export function TrainedModelsDeploymentModal({ ); setPendingDeployments(uniqueDeployments); // eslint-disable-next-line react-hooks/exhaustive-deps - }, [inferenceIdsInPendingList, inferenceToModelIdMap]); + }, [inferenceIdsInPendingList]); const erroredDeployments = pendingDeployments.filter( (deployment) => errorsInTrainedModelDeployment[deployment] From 2128a66075d9cf3f71bb6fb93728026ccd3ccf43 Mon Sep 17 00:00:00 2001 From: Samiul Monir Date: Wed, 6 Nov 2024 00:43:22 -0500 Subject: [PATCH 4/5] Removing model deployment from kibana - index management --- .../trained_models_deployment_modal.tsx | 42 +------------------ ..._details_page_mappings_model_management.ts | 6 +-- 2 files changed, 5 insertions(+), 43 deletions(-) diff --git a/x-pack/plugins/index_management/public/application/sections/home/index_list/details_page/trained_models_deployment_modal.tsx b/x-pack/plugins/index_management/public/application/sections/home/index_list/details_page/trained_models_deployment_modal.tsx index 4b41bcf139d88..b3dc5244165fb 100644 --- a/x-pack/plugins/index_management/public/application/sections/home/index_list/details_page/trained_models_deployment_modal.tsx +++ b/x-pack/plugins/index_management/public/application/sections/home/index_list/details_page/trained_models_deployment_modal.tsx @@ -26,10 +26,8 @@ import React from 'react'; import { EuiLink } from '@elastic/eui'; import { useEffect, useMemo, useState } from 'react'; import { i18n } from '@kbn/i18n'; -import { ModelIdMapEntry } from '../../../../components/mappings_editor/components/document_fields/fields'; import { isSemanticTextField } from '../../../../components/mappings_editor/lib/utils'; import { deNormalize } from '../../../../components/mappings_editor/lib'; -import { useMLModelNotificationToasts } from '../../../../../hooks/use_ml_model_status_toasts'; import { useMappingsState } from '../../../../components/mappings_editor/mappings_state_context'; import { useAppContext } from '../../../../app_context'; @@ -55,15 +53,11 @@ export function TrainedModelsDeploymentModal({ }: TrainedModelsDeploymentModalProps) { const modalTitleId = useGeneratedHtmlId(); const { fields, inferenceToModelIdMap } = useMappingsState(); - const { - plugins: { ml }, - url, - } = useAppContext(); + const { url } = useAppContext(); const [isModalVisible, setIsModalVisible] = useState(false); const closeModal = () => setIsModalVisible(false); const [mlManagementPageUrl, setMlManagementPageUrl] = useState(''); const [allowForceSaveMappings, setAllowForceSaveMappings] = useState(false); - const { showErrorToasts, showSuccessfullyDeployedToast } = useMLModelNotificationToasts(); useEffect(() => { const mlLocator = url?.locators.get(ML_APP_LOCATOR); @@ -86,25 +80,6 @@ export function TrainedModelsDeploymentModal({ const [pendingDeployments, setPendingDeployments] = useState([]); - const startModelAllocation = async (entry: ModelIdMapEntry & { inferenceId: string }) => { - try { - await ml?.mlApi?.trainedModels.startModelAllocation(entry.trainedModelId, { - number_of_allocations: 1, - threads_per_allocation: 1, - priority: 'normal', - deployment_id: entry.inferenceId, - }); - showSuccessfullyDeployedToast(entry.trainedModelId); - } catch (error) { - setErrorsInTrainedModelDeployment((previousState) => ({ - ...previousState, - [entry.inferenceId]: error.message, - })); - showErrorToasts(error); - setIsModalVisible(true); - } - }; - useEffect(() => { const models = inferenceIdsInPendingList.map((inferenceId) => inferenceToModelIdMap?.[inferenceId] @@ -114,18 +89,6 @@ export function TrainedModelsDeploymentModal({ } : undefined ); // filter out third-party models - for (const model of models) { - if ( - model?.trainedModelId && - model.isDeployable && - !model.isDownloading && - !model.isDeployed - ) { - // Sometimes the model gets stuck in a ready to deploy state, so we need to trigger deployment manually - // This is currently the only way to surface a specific error message to the user - startModelAllocation(model); - } - } const allPendingDeployments = models .map((model) => { return model?.trainedModelId && !model?.isDeployed ? model?.inferenceId : ''; @@ -135,8 +98,7 @@ export function TrainedModelsDeploymentModal({ (deployment, index) => allPendingDeployments.indexOf(deployment) === index ); setPendingDeployments(uniqueDeployments); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [inferenceIdsInPendingList]); + }, [inferenceIdsInPendingList, inferenceToModelIdMap]); const erroredDeployments = pendingDeployments.filter( (deployment) => errorsInTrainedModelDeployment[deployment] diff --git a/x-pack/plugins/index_management/public/hooks/use_details_page_mappings_model_management.ts b/x-pack/plugins/index_management/public/hooks/use_details_page_mappings_model_management.ts index 7a8d9f526b9f0..9ad3e74e96a9e 100644 --- a/x-pack/plugins/index_management/public/hooks/use_details_page_mappings_model_management.ts +++ b/x-pack/plugins/index_management/public/hooks/use_details_page_mappings_model_management.ts @@ -34,9 +34,9 @@ const getCustomInferenceIdMap = ( ? { trainedModelId: model.service_settings.model_id, isDeployable: model.service === Service.elser || model.service === Service.elasticsearch, - isDeployed: modelStatsById[model.inference_id]?.state === 'started', + isDeployed: modelStatsById[model.service_settings.model_id]?.state === 'started', isDownloading: Boolean(downloadStates[model.service_settings.model_id]), - modelStats: modelStatsById[model.inference_id], + modelStats: modelStatsById[model.service_settings.model_id], } : { trainedModelId: '', @@ -104,7 +104,7 @@ export const useDetailsPageMappingsModelManagement = () => { Record >((acc, { model_id: modelId, deployment_stats: stats }) => { if (modelId && stats) { - acc[stats.deployment_id] = stats; + acc[modelId] = stats; } return acc; }, {}) || {}; From 7ff26495e125f787121149b7072268e826aff5ab Mon Sep 17 00:00:00 2001 From: Samiul Monir Date: Wed, 6 Nov 2024 08:30:04 -0500 Subject: [PATCH 5/5] Reverting back to deployment id to point to inference id instead of model id --- .../hooks/use_details_page_mappings_model_management.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/index_management/public/hooks/use_details_page_mappings_model_management.ts b/x-pack/plugins/index_management/public/hooks/use_details_page_mappings_model_management.ts index 9ad3e74e96a9e..7a8d9f526b9f0 100644 --- a/x-pack/plugins/index_management/public/hooks/use_details_page_mappings_model_management.ts +++ b/x-pack/plugins/index_management/public/hooks/use_details_page_mappings_model_management.ts @@ -34,9 +34,9 @@ const getCustomInferenceIdMap = ( ? { trainedModelId: model.service_settings.model_id, isDeployable: model.service === Service.elser || model.service === Service.elasticsearch, - isDeployed: modelStatsById[model.service_settings.model_id]?.state === 'started', + isDeployed: modelStatsById[model.inference_id]?.state === 'started', isDownloading: Boolean(downloadStates[model.service_settings.model_id]), - modelStats: modelStatsById[model.service_settings.model_id], + modelStats: modelStatsById[model.inference_id], } : { trainedModelId: '', @@ -104,7 +104,7 @@ export const useDetailsPageMappingsModelManagement = () => { Record >((acc, { model_id: modelId, deployment_stats: stats }) => { if (modelId && stats) { - acc[modelId] = stats; + acc[stats.deployment_id] = stats; } return acc; }, {}) || {};