[ML] Sync ML saved objects to all spaces #202175
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…dyelastic/kibana into sync-ML-saved-objects-to-all-spaces
jgowdyelastic
added
release_note:enhancement
:ml
Feature:Anomaly Detection
|
Pinging @elastic/ml-ui (:ml) |
|
|
||
| router.versioned | ||
| .get({ | ||
| path: `${ML_INTERNAL_BASE_PATH}/saved_objects/can_sync_to_all_spaces/{mlSavedObjectType?}`, |
There was a problem hiding this comment.
Is it straightforward to add a dedicated test for this new endpoint?
darnautov
reviewed
Dec 24, 2024
| @@ -37,17 +38,22 @@ export const JobSpacesSyncFlyout: FC<Props> = ({ onClose }) => { | |||
| const { displayErrorToast, displaySuccessToast } = useToastNotificationService(); | |||
| const [loading, setLoading] = useState(false); | |||
| const [canSync, setCanSync] = useState(false); | |||
| const [canSyncToAllSpaces, setCanSyncToAllSpaces] = useState(true); | |||
There was a problem hiding this comment.
should it be false by default?
There was a problem hiding this comment.
The warning callout is shown when canSyncToAllSpaces is false. So by having it false by default it'd mean the warning would appear briefly while loading.
There was a problem hiding this comment.
shall we check for canSyncToAllSpaces && !loading?
There was a problem hiding this comment.
I think the initial value for canSyncToAllSpaces is fine as true. When the value is first used for the sync check, the check is in "simulate" mode where canSyncToAllSpaces is ignored.
When the user then runs the sync for real, canSyncToAllSpaces will have its correct value.
Also by adding a check for loading, it'd mean the warning will disappear when the user then clicks sync and reappear when the sync is complete.
x-pack/platform/plugins/shared/ml/public/application/services/ml_api_service/saved_objects.ts
Show resolved
Hide resolved
darnautov
approved these changes
Jan 6, 2025
💔 Build Failed
Failed CI Steps
Metrics [docs]Module Count
Async chunks
Public APIs missing exports
Page load bundle
History
|
|
Starting backport for target branches: 8.x |
kibanamachine
pushed a commit
to kibanamachine/kibana
that referenced
this pull request
Jan 7, 2025
When manually syncing ML saved objects using the sync flyout, the saved objects are now tagged to the `*` space. This now matches the behaviour of the server side auto sync and the sync which happens when the trained models page is loaded. The trained models page load sync has been extended to the AD and DA jobs lists and the overview page. If the user does not have write permission for ML in every space they cannot sync jobs to the `*` space. In this situation a warning is shown in the flyout and when they sync, the jobs/models will only be added to the current space.  (cherry picked from commit 3d65e89)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
kibanamachine
added a commit
that referenced
this pull request
Jan 7, 2025
# Backport This will backport the following commits from `main` to `8.x`: - [[ML] Sync ML saved objects to all spaces (#202175)](#202175) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"James Gowdy","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-01-07T09:56:00Z","message":"[ML] Sync ML saved objects to all spaces (#202175)\n\nWhen manually syncing ML saved objects using the sync flyout, the saved\r\nobjects are now tagged to the `*` space. This now matches the behaviour\r\nof the server side auto sync and the sync which happens when the trained\r\nmodels page is loaded.\r\nThe trained models page load sync has been extended to the AD and DA\r\njobs lists and the overview page.\r\n\r\nIf the user does not have write permission for ML in every space they\r\ncannot sync jobs to the `*` space.\r\nIn this situation a warning is shown in the flyout and when they sync,\r\nthe jobs/models will only be added to the current space.\r\n\r\n\r\n","sha":"3d65e892a014aa5a027d82caa9d92392a515390b","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:enhancement",":ml","Feature:Anomaly Detection","Feature:Data Frame Analytics","v9.0.0","Feature:3rd Party Models","backport:version","v8.18.0"],"title":"[ML] Sync ML saved objects to all spaces","number":202175,"url":"https://github.com/elastic/kibana/pull/202175","mergeCommit":{"message":"[ML] Sync ML saved objects to all spaces (#202175)\n\nWhen manually syncing ML saved objects using the sync flyout, the saved\r\nobjects are now tagged to the `*` space. This now matches the behaviour\r\nof the server side auto sync and the sync which happens when the trained\r\nmodels page is loaded.\r\nThe trained models page load sync has been extended to the AD and DA\r\njobs lists and the overview page.\r\n\r\nIf the user does not have write permission for ML in every space they\r\ncannot sync jobs to the `*` space.\r\nIn this situation a warning is shown in the flyout and when they sync,\r\nthe jobs/models will only be added to the current space.\r\n\r\n\r\n","sha":"3d65e892a014aa5a027d82caa9d92392a515390b"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/202175","number":202175,"mergeCommit":{"message":"[ML] Sync ML saved objects to all spaces (#202175)\n\nWhen manually syncing ML saved objects using the sync flyout, the saved\r\nobjects are now tagged to the `*` space. This now matches the behaviour\r\nof the server side auto sync and the sync which happens when the trained\r\nmodels page is loaded.\r\nThe trained models page load sync has been extended to the AD and DA\r\njobs lists and the overview page.\r\n\r\nIf the user does not have write permission for ML in every space they\r\ncannot sync jobs to the `*` space.\r\nIn this situation a warning is shown in the flyout and when they sync,\r\nthe jobs/models will only be added to the current space.\r\n\r\n\r\n","sha":"3d65e892a014aa5a027d82caa9d92392a515390b"}},{"branch":"8.x","label":"v8.18.0","branchLabelMappingKey":"^v8.18.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: James Gowdy <[email protected]>
kowalczyk-krzysztof
pushed a commit
to kowalczyk-krzysztof/kibana
that referenced
this pull request
Jan 7, 2025
When manually syncing ML saved objects using the sync flyout, the saved objects are now tagged to the `*` space. This now matches the behaviour of the server side auto sync and the sync which happens when the trained models page is loaded. The trained models page load sync has been extended to the AD and DA jobs lists and the overview page. If the user does not have write permission for ML in every space they cannot sync jobs to the `*` space. In this situation a warning is shown in the flyout and when they sync, the jobs/models will only be added to the current space. 
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When manually syncing ML saved objects using the sync flyout, the saved objects are now tagged to the
*space. This now matches the behaviour of the server side auto sync and the sync which happens when the trained models page is loaded.The trained models page load sync has been extended to the AD and DA jobs lists and the overview page.
If the user does not have write permission for ML in every space they cannot sync jobs to the
*space.In this situation a warning is shown in the flyout and when they sync, the jobs/models will only be added to the current space.