[ML] Trained Models: Prevents download of models already present in other spaces and shows warning by rbrtj · Pull Request #220238 · elastic/kibana

rbrtj · 2025-05-06T12:39:01Z

Resolves #206834
Before:

If a trained model like ELSER has already been downloaded but is not available in the current space, the download action is available but results in the following error

This PR prevents models from being downloaded if they already exist in a different space

If the model is downloaded in a different space and the user has write permissions for the current space:

If the model is downloaded in a different space but the user lacks write permissions for the current space:

Note:
It adds an additional request to ES, but within a trace, it is just one extra span, so the performance impact is not significant:

I tried adding functional tests for this, but it only applies to hosted models, which are too large to download durning the testing phase, thus I don't see a convenient way to test it.

…d display an appropriate warning

szabosteve · 2025-05-08T07:03:49Z

x-pack/platform/plugins/shared/ml/public/application/model_management/get_model_state.tsx

+                'xpack.ml.trainedModels.modelsList.modelState.downloadedInDifferentSpaceTooltip.cannotDeploy',
+                {
+                  defaultMessage:
+                    'The model is downloaded in a different space. Assign it to the current space see more details.',


I have two suggestions: a shorter one and a more user-friendly.

The shorter one:

Suggested change

'The model is downloaded in a different space. Assign it to the current space see more details.',

'The model is downloaded in a different space. Assign it to the current space to review model details.',

The more helpful:

Suggested change

'The model is downloaded in a different space. Assign it to the current space see more details.',

'The model is downloaded in a different space. If you have permission, assign it to the current space to review model details, or contact your administrator.',

The second one would be better. But it depends on how terrible it looks in the warning bubble.

Not that bad, but it takes some space 🤔

Not that bad. I edited my suggestion and made it even a bit shorter.

Yeah, I think that's good.

elasticmachine · 2025-05-08T08:52:58Z

Pinging @elastic/ml-ui (:ml)

peteharverson · 2025-05-08T10:23:43Z

...ck/platform/plugins/shared/ml/public/application/model_management/model_status_indicator.tsx

 import type { NameOverrides } from './get_model_state';
 import { getModelStateColor } from './get_model_state';
 import { useMlKibana } from '../contexts/kibana';
+import { usePermissionCheck } from '../capabilities/check_capabilities';


Haven't tested if this happens on main too but hit this error if:

From Default space, deploy the e5 model.

In default space, stop the deployment

Then from the default, or from a different space, try and run 'Start deployment'

Haven't tested if this happens on main too

Probably not, as the capabilities check throws an error here, let me investigate - maybe I misused the capabilities check 🤔

Haven't been able to reproduce this on main.

Tested against 0981f16 and all looks good now.

jgowdyelastic · 2025-05-08T10:38:09Z

x-pack/platform/plugins/shared/ml/server/models/model_management/models_provider.ts

  ): Promise<TrainedModelUIItem[]> {
    const { trainedModelsSpaces } = checksFactory(this._client, mlSavedObjectService);

+    const rawModels = await this._client.asInternalUser.ml.getTrainedModels({


I think we should be using asCurrentUser here.
By using internal user, we might incorrectly flag a model as being downloaded to a different space, when really the user does not have permission to get the models at all.

👍 Done in 55c4d06

x-pack/platform/plugins/shared/ml/server/models/model_management/models_provider.ts

peteharverson

Tested latest changes and LGTM

jgowdyelastic · 2025-05-08T16:19:09Z

x-pack/platform/plugins/shared/ml/server/models/model_management/models_provider.ts

  ): Promise<TrainedModelUIItem[]> {
    const { trainedModelsSpaces } = checksFactory(this._client, mlSavedObjectService);

+    const rawModels = await this._client.asCurrentUser.ml.getTrainedModels({


I wonder whether this should be moved inside includeModelDownloads as that is the only place it is used and it is already an async function

good point, it is definitely worth moving it into includeModelDownloads
done in 96babd0

Updated to use Promise.all for rawModels and forDownload models.
860fb19

jgowdyelastic

LGTM

elasticmachine · 2025-05-09T10:12:29Z

⏳ Build in-progress, with failures

Buildkite Build
Commit: 860fb19

Failed CI Steps

Test Failures

[job] [logs] FTR Configs #97 / Cloud Security Posture Cloud Posture Rules Page - Table Headers Rules Page - Enable Rules and Disabled Rules Filter Toggle Should only display Enabled rules when Enabled Rules filter is ON
[job] [logs] FTR Configs #97 / Cloud Security Posture Vulnerabilities Page - DataTable "before all" hook in "Vulnerabilities Page - DataTable"

History

💛 Build #299723 was flaky 0981f16
💔 Build #299671 failed 18c5a55
💔 Build #299635 failed d9e4719

cc @rbrtj

kibanamachine · 2025-05-09T21:53:32Z

Starting backport for target branches: 8.19

https://github.com/elastic/kibana/actions/runs/14938348953

…her Spaces with Warning (elastic#220238) Resolves elastic#206834 Before: > If a trained model like ELSER has already been downloaded but is not available in the current space, the download action is available but results in the following error ![image](https://github.com/user-attachments/assets/e69516e5-749e-4500-b645-099975453f1e) This PR prevents models from being downloaded if they already exist in a different space * If the model is downloaded in a different space and the user has write permissions for the current space: <img width="1228" alt="image" src="https://github.com/user-attachments/assets/db146bbb-a295-471e-b0a1-f1a8c0949739" /> * If the model is downloaded in a different space but the user lacks write permissions for the current space: <img width="1479" alt="image" src="https://github.com/user-attachments/assets/dcf195c1-7314-4b0b-af9d-1fbbaffcbb89" /> Note: It adds an additional request to ES, but within a trace, it is just one extra span, so the performance impact is not significant: ![image](https://github.com/user-attachments/assets/8be08344-eb91-4cdb-a5b1-a4838f5219b6) I tried adding functional tests for this, but it only applies to hosted models, which are too large to download durning the testing phase, thus I don't see a convenient way to test it. (cherry picked from commit c97f2c7)

kibanamachine · 2025-05-09T21:59:48Z

💚 All backports created successfully

Status	Branch	Result
✅	8.19

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

…t in Other Spaces with Warning (#220238) (#220723) # Backport This will backport the following commits from `main` to `8.19`: - [[ML] Trained Models: Prevent Download of Models Already Present in Other Spaces with Warning (#220238)](#220238)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport)  Co-authored-by: Robert Jaszczurek <92210485+rbrtj@users.noreply.github.com>

…her Spaces with Warning (elastic#220238) Resolves elastic#206834 Before: > If a trained model like ELSER has already been downloaded but is not available in the current space, the download action is available but results in the following error ![image](https://github.com/user-attachments/assets/e69516e5-749e-4500-b645-099975453f1e) This PR prevents models from being downloaded if they already exist in a different space * If the model is downloaded in a different space and the user has write permissions for the current space: <img width="1228" alt="image" src="https://github.com/user-attachments/assets/db146bbb-a295-471e-b0a1-f1a8c0949739" /> * If the model is downloaded in a different space but the user lacks write permissions for the current space: <img width="1479" alt="image" src="https://github.com/user-attachments/assets/dcf195c1-7314-4b0b-af9d-1fbbaffcbb89" /> Note: It adds an additional request to ES, but within a trace, it is just one extra span, so the performance impact is not significant: ![image](https://github.com/user-attachments/assets/8be08344-eb91-4cdb-a5b1-a4838f5219b6) I tried adding functional tests for this, but it only applies to hosted models, which are too large to download durning the testing phase, thus I don't see a convenient way to test it.

rbrtj added 2 commits May 6, 2025 14:37

Prevent downloading models already downloaded in a different space an…

9688708

…d display an appropriate warning

provide different messages for read and write user access

d84f44c

rbrtj changed the title ~~Prevent downloading models already downloaded in a different space an…~~ May 7, 2025

szabosteve reviewed May 8, 2025

View reviewed changes

update warning message

d9e4719

rbrtj marked this pull request as ready for review May 8, 2025 08:51

rbrtj requested a review from a team as a code owner May 8, 2025 08:51

rbrtj requested review from jgowdyelastic and peteharverson May 8, 2025 08:51

rbrtj self-assigned this May 8, 2025

rbrtj added release_note:enhancement :ml Feature:3rd Party Models ML 3rd party models Team:ML Team label for ML (also use :ml) t// backport:version Backport to applied version labels v9.1.0 v8.19.0 labels May 8, 2025

rbrtj requested a review from szabosteve May 8, 2025 09:01

update message

eeb3b71

peteharverson reviewed May 8, 2025

View reviewed changes

Merge branch 'main' into ml-trained-models-improve-messaging-on-download

18c5a55

jgowdyelastic reviewed May 8, 2025

View reviewed changes

rbrtj added 2 commits May 8, 2025 14:29

use asCurrentUser instead of internal

55c4d06

update context dependencies

0981f16

jgowdyelastic reviewed May 8, 2025

View reviewed changes

x-pack/platform/plugins/shared/ml/server/models/model_management/models_provider.ts Outdated Show resolved Hide resolved

peteharverson approved these changes May 8, 2025

View reviewed changes

jgowdyelastic reviewed May 8, 2025

View reviewed changes

move raw models es call into includeModelDownloads

96babd0

rbrtj requested a review from jgowdyelastic May 9, 2025 08:27

wrap calls with Promise.all

860fb19

jgowdyelastic approved these changes May 9, 2025

View reviewed changes

Merge branch 'main' into ml-trained-models-improve-messaging-on-download

db40530

rbrtj merged commit c97f2c7 into elastic:main May 9, 2025
9 checks passed

kibanamachine mentioned this pull request May 9, 2025

[8.19] [ML] Trained Models: Prevent Download of Models Already Present in Other Spaces with Warning (#220238) #220723

Merged

nastasha-solomon changed the title ~~[ML] Trained Models: Prevent Download of Models Already Present in Other Spaces with Warning~~ Jun 20, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ML] Trained Models: Prevents download of models already present in other spaces and shows warning#220238

[ML] Trained Models: Prevents download of models already present in other spaces and shows warning#220238
rbrtj merged 10 commits intoelastic:mainfrom
rbrtj:ml-trained-models-improve-messaging-on-download

rbrtj commented May 6, 2025 •

edited by kibanamachine

Loading

szabosteve May 8, 2025 •

edited

Loading

rbrtj May 8, 2025 •

edited

Loading

szabosteve May 8, 2025

rbrtj May 8, 2025

szabosteve May 8, 2025

elasticmachine commented May 8, 2025

peteharverson May 8, 2025

rbrtj May 8, 2025

peteharverson May 8, 2025

peteharverson May 8, 2025

jgowdyelastic May 8, 2025

rbrtj May 8, 2025

Uh oh!

peteharverson left a comment

jgowdyelastic May 8, 2025 •

edited

Loading

rbrtj May 9, 2025

rbrtj May 9, 2025

jgowdyelastic left a comment

elasticmachine commented May 9, 2025

Uh oh!

kibanamachine commented May 9, 2025

kibanamachine commented May 9, 2025

Labels

6 participants

	'The model is downloaded in a different space. Assign it to the current space see more details.',
	'The model is downloaded in a different space. Assign it to the current space to review model details.',

Conversation

rbrtj commented May 6, 2025 • edited by kibanamachine Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

szabosteve May 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

rbrtj May 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented May 8, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Uh oh!

peteharverson left a comment

Choose a reason for hiding this comment

jgowdyelastic May 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jgowdyelastic left a comment

Choose a reason for hiding this comment

elasticmachine commented May 9, 2025

⏳ Build in-progress, with failures

Failed CI Steps

Test Failures

History

Uh oh!

kibanamachine commented May 9, 2025

kibanamachine commented May 9, 2025

💚 All backports created successfully

Questions ?

Labels

6 participants

rbrtj commented May 6, 2025 •

edited by kibanamachine

Loading

szabosteve May 8, 2025 •

edited

Loading

rbrtj May 8, 2025 •

edited

Loading

jgowdyelastic May 8, 2025 •

edited

Loading