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] Trained models: Replace download button by extending deploy action #205699

Open
wants to merge 36 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
0de3198
introducing trained models service to manage models download & deploy…
rbrtj Jan 2, 2025
d0078f9
rename action to Start Deployment
rbrtj Jan 2, 2025
1474865
add a queue for deployments started before the model is downloaded
rbrtj Jan 2, 2025
0d52bc0
rename State column to Model state
rbrtj Jan 2, 2025
1727e2c
model actions cleanup
rbrtj Jan 2, 2025
6f96af1
rename Delete action to Delete Model && update actions order
rbrtj Jan 2, 2025
cf1ca47
trained models service small refactor
rbrtj Jan 7, 2025
31c18e8
align error handling with previous implementation
rbrtj Jan 7, 2025
3b646de
fix trained models service flow to correctly keep track of active ope…
rbrtj Jan 7, 2025
0c0eadf
Merge remote-tracking branch 'upstream/main' into trained-models-repl…
rbrtj Jan 9, 2025
2646bef
fix loading state in trained models service
rbrtj Jan 9, 2025
b9d6c5f
ensure user can start one deployment at a time && isLoading based on …
rbrtj Jan 9, 2025
173321f
refactor trained models service and corresponding components to use m…
rbrtj Jan 10, 2025
c079ddf
cancel all request when user deletes a model after starting a download
rbrtj Jan 10, 2025
9856fe7
use withLatestFrom for polling logic
rbrtj Jan 10, 2025
27f8d9f
remove isInitialized
rbrtj Jan 10, 2025
5acf8dd
use trained models service hook - improve singletone behavior && useU…
rbrtj Jan 13, 2025
8847920
fix trained models list actions and expanding row icon buttons overla…
rbrtj Jan 13, 2025
11b951e
display deploying state
rbrtj Jan 14, 2025
a81df16
simplify startModelDeployment$
rbrtj Jan 14, 2025
7863d66
update breakpoint
rbrtj Jan 14, 2025
f73f9df
increase actions column width
rbrtj Jan 14, 2025
ac599eb
revert startModelDeployment changes
rbrtj Jan 14, 2025
ea1800a
fix requests cancelation
rbrtj Jan 14, 2025
edac0af
remove comment
rbrtj Jan 14, 2025
b3f47a5
use ModelStatusIndicator in models_list
rbrtj Jan 14, 2025
06debd4
Merge branch 'main' into trained-models-replace-download-button-by-ex…
rbrtj Jan 14, 2025
c23ad1d
[CI] Auto-commit changed files from 'node scripts/yarn_deduplicate'
kibanamachine Jan 14, 2025
630e994
remove unused translations
rbrtj Jan 14, 2025
832418b
display success toast after deploying the model
rbrtj Jan 15, 2025
60129c2
Merge branch 'main' into trained-models-replace-download-button-by-ex…
rbrtj Jan 16, 2025
62b3a61
update start deployment toast messages
rbrtj Jan 21, 2025
a199de2
deployment modal: add divider, allow progress bar to take more space
rbrtj Jan 21, 2025
e651ce4
show only downloading and ready for deployment states in deployment m…
rbrtj Jan 21, 2025
0a6ff9f
update progress label inside deployment modal && update label color
rbrtj Jan 21, 2025
2399126
trained models service refactor
rbrtj Jan 30, 2025
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
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,18 @@ const navigateToUrl = jest.fn().mockImplementation(async (url) => {
describe('useUnsavedChangesPrompt', () => {
let addSpy: jest.SpiedFunction<Window['addEventListener']>;
let removeSpy: jest.SpiedFunction<Window['removeEventListener']>;
let blockSpy: jest.SpiedFunction<CoreScopedHistory['block']>;

beforeEach(() => {
addSpy = jest.spyOn(window, 'addEventListener');
removeSpy = jest.spyOn(window, 'removeEventListener');
blockSpy = jest.spyOn(history, 'block');
});

afterEach(() => {
addSpy.mockRestore();
removeSpy.mockRestore();
blockSpy.mockRestore();
jest.resetAllMocks();
});

Expand Down Expand Up @@ -97,4 +100,23 @@ describe('useUnsavedChangesPrompt', () => {
expect(addSpy).toBeCalledWith('beforeunload', expect.anything());
expect(removeSpy).toBeCalledWith('beforeunload', expect.anything());
});

it('should not block SPA navigation if blockSpaNavigation is false', async () => {
renderHook(() =>
useUnsavedChangesPrompt({
hasUnsavedChanges: true,
blockSpaNavigation: false,
})
);

expect(addSpy).toBeCalledWith('beforeunload', expect.anything());

act(() => history.push('/test'));

expect(coreStart.overlays.openConfirm).not.toBeCalled();

expect(history.location.pathname).toBe('/test');

expect(blockSpy).not.toBeCalled();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,11 @@ const DEFAULT_CONFIRM_BUTTON = i18n.translate('unsavedChangesPrompt.defaultModal
defaultMessage: 'Leave page',
});

interface Props {
interface BaseProps {
hasUnsavedChanges: boolean;
}

interface SpaBlockingProps extends BaseProps {
http: HttpStart;
openConfirm: OverlayStart['openConfirm'];
history: ScopedHistory;
Expand All @@ -38,20 +41,21 @@ interface Props {
messageText?: string;
cancelButtonText?: string;
confirmButtonText?: string;
blockSpaNavigation?: true;
}

interface BrowserBlockingProps extends BaseProps {
blockSpaNavigation: false;
}

export const useUnsavedChangesPrompt = ({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: This is just a proposal, so it is possible to use useUnsavedChangesPrompt without blocking SPA navigation. If you don't like the implementation, we can separate the relevant part of the hook into our package

hasUnsavedChanges,
openConfirm,
history,
http,
navigateToUrl,
// Provide overrides for confirm dialog
messageText = DEFAULT_BODY_TEXT,
titleText = DEFAULT_TITLE_TEXT,
confirmButtonText = DEFAULT_CONFIRM_BUTTON,
cancelButtonText = DEFAULT_CANCEL_BUTTON,
}: Props) => {
type Props = SpaBlockingProps | BrowserBlockingProps;

const isSpaBlocking = (props: Props): props is SpaBlockingProps =>
props.blockSpaNavigation !== false;

export const useUnsavedChangesPrompt = (props: Props) => {
const { hasUnsavedChanges, blockSpaNavigation = true } = props;

useEffect(() => {
if (hasUnsavedChanges) {
const handler = (event: BeforeUnloadEvent) => {
Expand All @@ -67,10 +71,22 @@ export const useUnsavedChangesPrompt = ({
}, [hasUnsavedChanges]);

useEffect(() => {
if (!hasUnsavedChanges) {
if (!hasUnsavedChanges || !isSpaBlocking(props)) {
return;
}

const {
openConfirm,
http,
history,
navigateToUrl,
// Provide overrides for confirm dialog
messageText = DEFAULT_BODY_TEXT,
titleText = DEFAULT_TITLE_TEXT,
confirmButtonText = DEFAULT_CONFIRM_BUTTON,
cancelButtonText = DEFAULT_CANCEL_BUTTON,
} = props;

const unblock = history.block((state) => {
async function confirmAsync() {
const confirmResponse = await openConfirm(messageText, {
Expand All @@ -97,15 +113,5 @@ export const useUnsavedChangesPrompt = ({
});

return unblock;
}, [
history,
hasUnsavedChanges,
openConfirm,
navigateToUrl,
http.basePath,
titleText,
cancelButtonText,
confirmButtonText,
messageText,
]);
}, [hasUnsavedChanges, blockSpaNavigation, props]);
};
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,21 @@ import type { CoreStart, OverlayStart } from '@kbn/core/public';
import { css } from '@emotion/react';
import { toMountPoint } from '@kbn/react-kibana-mount';
import { dictionaryValidator } from '@kbn/ml-validators';
import { KibanaContextProvider } from '@kbn/kibana-react-plugin/public';
import type { NLPSettings } from '../../../common/constants/app';
import type {
NLPModelItem,
TrainedModelDeploymentStatsResponse,
import type { ModelDownloadItem } from '../../../common/types/trained_models';
import {
isModelDownloadItem,
type NLPModelItem,
type TrainedModelDeploymentStatsResponse,
} from '../../../common/types/trained_models';
import { type CloudInfo, getNewJobLimits } from '../services/ml_server_info';
import type { MlStartTrainedModelDeploymentRequestNew } from './deployment_params_mapper';
import { DeploymentParamsMapper } from './deployment_params_mapper';

import type { HttpService } from '../services/http_service';
import { ModelStatusIndicator } from './model_status_indicator';

interface DeploymentSetupProps {
config: DeploymentParamsUI;
onConfigChange: (config: DeploymentParamsUI) => void;
Expand Down Expand Up @@ -647,7 +653,7 @@ export const DeploymentSetup: FC<DeploymentSetupProps> = ({
};

interface StartDeploymentModalProps {
model: NLPModelItem;
model: NLPModelItem | ModelDownloadItem;
startModelDeploymentDocUrl: string;
onConfigChange: (config: DeploymentParamsUI) => void;
onClose: () => void;
Expand Down Expand Up @@ -676,27 +682,35 @@ export const StartUpdateDeploymentModal: FC<StartDeploymentModalProps> = ({
}) => {
const isUpdate = !!initialParams;

const isModelNotDownloaded = isModelDownloadItem(model);

const getDefaultParams = useCallback((): DeploymentParamsUI => {
const defaultVCPUUsage: DeploymentParamsUI['vCPUUsage'] = showNodeInfo ? 'medium' : 'low';

const defaultParams = {
deploymentId: `${model.model_id}_ingest`,
optimized: 'optimizedForIngest',
vCPUUsage: defaultVCPUUsage,
adaptiveResources: true,
} as const;

if (isModelNotDownloaded) {
return defaultParams;
}

const uiParams = model.stats?.deployment_stats.map((v) =>
deploymentParamsMapper.mapApiToUiDeploymentParams(v)
);

const defaultVCPUUsage: DeploymentParamsUI['vCPUUsage'] = showNodeInfo ? 'medium' : 'low';

return uiParams?.some((v) => v.optimized === 'optimizedForIngest')
? {
deploymentId: `${model.model_id}_search`,
optimized: 'optimizedForSearch',
vCPUUsage: defaultVCPUUsage,
adaptiveResources: true,
}
: {
deploymentId: `${model.model_id}_ingest`,
optimized: 'optimizedForIngest',
vCPUUsage: defaultVCPUUsage,
adaptiveResources: true,
};
}, [deploymentParamsMapper, model.model_id, model.stats?.deployment_stats, showNodeInfo]);
: defaultParams;
}, [deploymentParamsMapper, isModelNotDownloaded, model, showNodeInfo]);

const [config, setConfig] = useState<DeploymentParamsUI>(initialParams ?? getDefaultParams());

Expand All @@ -709,12 +723,12 @@ export const StartUpdateDeploymentModal: FC<StartDeploymentModalProps> = ({
otherModelAndDeploymentIds.splice(otherModelAndDeploymentIds?.indexOf(model.model_id), 1);

return dictionaryValidator([
...model.deployment_ids,
...(isModelNotDownloaded ? [] : model.deployment_ids),
...otherModelAndDeploymentIds,
// check for deployment with the default ID
...(model.deployment_ids.includes(model.model_id) ? [''] : []),
...(isModelNotDownloaded ? [] : model.deployment_ids.includes(model.model_id) ? [''] : []),
]);
}, [modelAndDeploymentIds, model.deployment_ids, model.model_id, isUpdate]);
}, [modelAndDeploymentIds, model, isUpdate, isModelNotDownloaded]);

const deploymentIdErrors = deploymentIdValidator(config.deploymentId ?? '');

Expand All @@ -725,21 +739,28 @@ export const StartUpdateDeploymentModal: FC<StartDeploymentModalProps> = ({
return (
<EuiModal onClose={onClose} data-test-subj="mlModelsStartDeploymentModal" maxWidth={640}>
<EuiModalHeader>
<EuiModalHeaderTitle size="s">
{isUpdate ? (
<FormattedMessage
id="xpack.ml.trainedModels.modelsList.updateDeployment.modalTitle"
defaultMessage="Update {modelId} deployment"
values={{ modelId: model.model_id }}
/>
) : (
<FormattedMessage
id="xpack.ml.trainedModels.modelsList.startDeployment.modalTitle"
defaultMessage="Start {modelId} deployment"
values={{ modelId: model.model_id }}
/>
)}
</EuiModalHeaderTitle>
<EuiFlexGroup direction="column" gutterSize="s">
<EuiFlexItem>
<EuiModalHeaderTitle size="s">
{isUpdate ? (
<FormattedMessage
id="xpack.ml.trainedModels.modelsList.updateDeployment.modalTitle"
defaultMessage="Update {modelId} deployment"
values={{ modelId: model.model_id }}
/>
) : (
<FormattedMessage
id="xpack.ml.trainedModels.modelsList.startDeployment.modalTitle"
defaultMessage="Start {modelId} deployment"
values={{ modelId: model.model_id }}
/>
)}
</EuiModalHeaderTitle>
</EuiFlexItem>
<EuiFlexItem>
<ModelStatusIndicator modelId={model.model_id} />
</EuiFlexItem>
</EuiFlexGroup>
</EuiModalHeader>

<EuiModalBody>
Expand All @@ -754,12 +775,18 @@ export const StartUpdateDeploymentModal: FC<StartDeploymentModalProps> = ({
disableAdaptiveResourcesControl={
showNodeInfo ? false : !nlpSettings.modelDeployment.allowStaticAllocations
}
deploymentsParams={model.stats?.deployment_stats.reduce<
Record<string, DeploymentParamsUI>
>((acc, curr) => {
acc[curr.deployment_id] = deploymentParamsMapper.mapApiToUiDeploymentParams(curr);
return acc;
}, {})}
deploymentsParams={
isModelNotDownloaded
? {}
: model.stats?.deployment_stats.reduce<Record<string, DeploymentParamsUI>>(
(acc, curr) => {
acc[curr.deployment_id] =
deploymentParamsMapper.mapApiToUiDeploymentParams(curr);
return acc;
},
{}
)
}
/>

<EuiHorizontalRule margin="m" />
Expand Down Expand Up @@ -844,7 +871,8 @@ export const getUserInputModelDeploymentParamsProvider =
startModelDeploymentDocUrl: string,
cloudInfo: CloudInfo,
showNodeInfo: boolean,
nlpSettings: NLPSettings
nlpSettings: NLPSettings,
httpService: HttpService
) =>
(
model: NLPModelItem,
Expand All @@ -867,24 +895,26 @@ export const getUserInputModelDeploymentParamsProvider =
try {
const modalSession = overlays.openModal(
toMountPoint(
<StartUpdateDeploymentModal
nlpSettings={nlpSettings}
showNodeInfo={showNodeInfo}
deploymentParamsMapper={deploymentParamsMapper}
cloudInfo={cloudInfo}
startModelDeploymentDocUrl={startModelDeploymentDocUrl}
initialParams={params}
modelAndDeploymentIds={deploymentIds}
model={model}
onConfigChange={(config) => {
modalSession.close();
resolve(deploymentParamsMapper.mapUiToApiDeploymentParams(config));
}}
onClose={() => {
modalSession.close();
resolve();
}}
/>,
<KibanaContextProvider services={{ mlServices: { httpService } }}>
<StartUpdateDeploymentModal
nlpSettings={nlpSettings}
showNodeInfo={showNodeInfo}
deploymentParamsMapper={deploymentParamsMapper}
cloudInfo={cloudInfo}
startModelDeploymentDocUrl={startModelDeploymentDocUrl}
initialParams={params}
modelAndDeploymentIds={deploymentIds}
model={model}
onConfigChange={(config) => {
modalSession.close();
resolve(deploymentParamsMapper.mapUiToApiDeploymentParams(config));
}}
onClose={() => {
modalSession.close();
resolve();
}}
/>
</KibanaContextProvider>,
startServices
)
);
Expand Down
Loading