[SLOs] Try async creation of resources !!#192836
Conversation
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
|
Pinging @elastic/obs-ux-management-team (Team:obs-ux-management) |
…into slo-create-perf
kdelemme
left a comment
There was a problem hiding this comment.
Parallelizing the operations is a good improvement, but I think the change in the repository is not acceptable for the reason cited in the comments.
Happy to discuss more.
I might have one idea if we really want to split the exist assertion and the save operation (but including the save operation in the promise.all).
kdelemme
left a comment
There was a problem hiding this comment.
Thanks for the changes, I think we can leverage react query callback functions in the UI. Going to test locally now
| return slo; | ||
| } | ||
|
|
||
| async update(slo: SLODefinition): Promise<SLODefinition> { |
There was a problem hiding this comment.
Technically this function can also create a new SLO 🙈
There was a problem hiding this comment.
i think we can live with that :D
kdelemme
left a comment
There was a problem hiding this comment.
One last comment about usage of the hook
kdelemme
left a comment
There was a problem hiding this comment.
Just need to fix the loadingToastId typing and usage but otherwise LGTM.
| return useMutation< | ||
| CreateRuleResponse<Params> & { loadingToastId?: string }, | ||
| Error, | ||
| { rule: CreateRuleRequestBody<Params> } | ||
| >( |
There was a problem hiding this comment.
loadingToastId is not part of the Data response. it belongs to the context, which is the 4th type parameter.
| return useMutation< | |
| CreateRuleResponse<Params> & { loadingToastId?: string }, | |
| Error, | |
| { rule: CreateRuleRequestBody<Params> } | |
| >( | |
| return useMutation< | |
| CreateRuleResponse<Params>, | |
| Error, | |
| { rule: CreateRuleRequestBody<Params> }, | |
| { loadingToastId: string } | |
| >( |
| onSettled: (data) => { | ||
| if (data?.loadingToastId) { | ||
| toasts.remove(data?.loadingToastId); | ||
| } | ||
| }, |
There was a problem hiding this comment.
I wonder how this works locally because loadingToastId is in the context variable, which is the 4th argument of the onSettled function: https://tanstack.com/query/v4/docs/framework/react/guides/mutations#mutation-side-effects
data is what is returned by the mutate query function (the API call basically).
| onSettled: (data) => { | |
| if (data?.loadingToastId) { | |
| toasts.remove(data?.loadingToastId); | |
| } | |
| }, | |
| onSettled: (_, _, _, context) => { | |
| if (context?.loadingToastId) { | |
| toasts.remove(context?.loadingToastId); | |
| } | |
| }, |
| rollbackOperations.push(() => this.transformManager.stop(rollupTransformId)); | ||
| rollbackOperations.push(() => this.summaryTransformManager.stop(summaryTransformId)); | ||
|
|
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: |
kdelemme
left a comment
There was a problem hiding this comment.
Thanks for the changes, tested locally and works as expected!
|
Starting backport for target branches: 8.x https://github.com/elastic/kibana/actions/runs/11128644789 |
## Summary Try async as much as possible while creating SLO !! Also UI form won't wait now for creating burn rate rule, it will be created async loading state in toast !! ### Before  ### After  (cherry picked from commit 9e117c3)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
# Backport This will backport the following commits from `main` to `8.x`: - [[SLOs] Try async creation of resources !! (#192836)](#192836) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Shahzad","email":"shahzad31comp@gmail.com"},"sourceCommit":{"committedDate":"2024-10-01T15:33:29Z","message":"[SLOs] Try async creation of resources !! (#192836)\n\n## Summary\r\n\r\nTry async as much as possible while creating SLO !!\r\n\r\nAlso UI form won't wait now for creating burn rate rule, it will be\r\ncreated async loading state in toast !!\r\n### Before\r\n\r\n\r\n\r\n\r\n### After\r\n\r\n\r\n","sha":"9e117c3aa22f3296cfda2d4f43c1729562290057","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","backport:prev-minor","ci:project-deploy-observability","Team:obs-ux-management"],"title":"[SLOs] Try async creation of resources !!","number":192836,"url":"https://github.com/elastic/kibana/pull/192836","mergeCommit":{"message":"[SLOs] Try async creation of resources !! (#192836)\n\n## Summary\r\n\r\nTry async as much as possible while creating SLO !!\r\n\r\nAlso UI form won't wait now for creating burn rate rule, it will be\r\ncreated async loading state in toast !!\r\n### Before\r\n\r\n\r\n\r\n\r\n### After\r\n\r\n\r\n","sha":"9e117c3aa22f3296cfda2d4f43c1729562290057"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/192836","number":192836,"mergeCommit":{"message":"[SLOs] Try async creation of resources !! (#192836)\n\n## Summary\r\n\r\nTry async as much as possible while creating SLO !!\r\n\r\nAlso UI form won't wait now for creating burn rate rule, it will be\r\ncreated async loading state in toast !!\r\n### Before\r\n\r\n\r\n\r\n\r\n### After\r\n\r\n\r\n","sha":"9e117c3aa22f3296cfda2d4f43c1729562290057"}}]}] BACKPORT--> Co-authored-by: Shahzad <shahzad31comp@gmail.com>
Summary
Try async as much as possible while creating SLO !!
Also UI form won't wait now for creating burn rate rule, it will be created async loading state in toast !!
Before
After