kraken: allow cancelling install/update operations#3857
kraken: allow cancelling install/update operations#3857nicoschmdt wants to merge 2 commits intobluerobotics:masterfrom
Conversation
Reviewer's GuideImplements cancellable install/update/finalize operations for Kraken extensions by wiring AbortController-based cancellation through the Vue frontend, Axios requests, and backend extension update/install semantics, while refining version handling and error reporting. Sequence diagram for cancellable extension install operationsequenceDiagram
actor User
participant ExtensionManagerView
participant PullProgress
participant KrakenManager
participant Axios
participant KrakenAPIv2
participant ExtensionService
User->>ExtensionManagerView: clickInstall(extension)
ExtensionManagerView->>ExtensionManagerView: beginInstallOperation()
note over ExtensionManagerView: Create AbortController and store as active_abort_controller
ExtensionManagerView->>KrakenManager: installExtension(extension, progressHandler, signal)
KrakenManager->>Axios: back_axios(POST /extension/install, signal)
Axios->>KrakenAPIv2: HTTP POST /extension/install (stream)
KrakenAPIv2->>ExtensionService: extension.install(clear_remaining_tags, atomic, should_enable=True)
ExtensionService-->>KrakenAPIv2: progress chunks
KrakenAPIv2-->>Axios: progress chunks
Axios-->>ExtensionManagerView: onDownloadProgress events
ExtensionManagerView->>ExtensionManagerView: handleDownloadProgress(event, tracker)
User->>PullProgress: clickCancel()
PullProgress-->>ExtensionManagerView: cancel
ExtensionManagerView->>ExtensionManagerView: cancelInstallOperation()
ExtensionManagerView->>ExtensionManagerView: active_abort_controller.abort()
ExtensionManagerView->>Axios: abort via signal
Axios-->>ExtensionManagerView: cancellationError (axios.isCancel)
alt cancellation
ExtensionManagerView->>KrakenManager: uninstallExtension(extension.identifier)
KrakenManager->>Axios: back_axios(DELETE /extension/{identifier})
Axios->>KrakenAPIv2: HTTP DELETE /extension/{identifier}
KrakenAPIv2->>ExtensionService: extension.uninstall()
ExtensionService-->>KrakenAPIv2: uninstall ok
KrakenAPIv2-->>Axios: 202 Accepted
Axios-->>ExtensionManagerView: uninstall ok
ExtensionManagerView->>ExtensionManagerView: notifier.pushInfo(EXTENSION_INSTALL_CANCELLED)
end
ExtensionManagerView->>ExtensionManagerView: finishInstallOperation()
note over ExtensionManagerView: active_abort_controller = null
ExtensionManagerView->>ExtensionManagerView: clearInstallingState()
ExtensionManagerView->>ExtensionManagerView: resetPullOutput()
ExtensionManagerView->>ExtensionManagerView: fetchInstalledExtensions()
Sequence diagram for cancellable extension update with version swapsequenceDiagram
actor User
participant ExtensionManagerView
participant KrakenManager
participant Axios
participant KrakenAPIv2
participant ExtensionService
User->>ExtensionManagerView: clickUpdate(extension, newTag)
ExtensionManagerView->>ExtensionManagerView: beginInstallOperation()
ExtensionManagerView->>KrakenManager: updateExtensionToVersion(identifier, newTag, progressHandler, signal)
KrakenManager->>Axios: back_axios(PUT /extension/{identifier}/{newTag}, params purge=false, should_enable=false, signal)
Axios->>KrakenAPIv2: HTTP PUT /extension/{identifier}/{tag}?purge=false&should_enable=false
KrakenAPIv2->>ExtensionService: extension.update(clear_remaining_tags=False, should_enable=False)
ExtensionService->>ExtensionService: install(clear_remaining_tags=False, atomic=False, should_enable=False)
ExtensionService->>ExtensionService: _create_extension_settings(should_enable=False)
ExtensionService-->>KrakenAPIv2: progress chunks
alt user cancels
User->>ExtensionManagerView: cancel via PullProgress
ExtensionManagerView->>ExtensionManagerView: cancelInstallOperation()
ExtensionManagerView->>Axios: abort via signal
Axios-->>ExtensionManagerView: cancellationError (axios.isCancel)
ExtensionManagerView->>ExtensionManagerView: swapExtensionVersion(identifier, previousTag, newTag)
ExtensionManagerView->>KrakenManager: enableExtension(identifier, previousTag)
ExtensionManagerView->>KrakenManager: uninstallExtensionVersion(identifier, newTag)
else success
Axios-->>ExtensionManagerView: 200 OK
ExtensionManagerView->>ExtensionManagerView: notifier.pushSuccess(EXTENSION_UPDATE_SUCCESS)
ExtensionManagerView->>ExtensionManagerView: swapExtensionVersion(identifier, newTag, previousTag)
ExtensionManagerView->>KrakenManager: enableExtension(identifier, newTag)
ExtensionManagerView->>KrakenManager: uninstallExtensionVersion(identifier, previousTag)
end
ExtensionManagerView->>ExtensionManagerView: finishInstallOperation()
Class diagram for updated Kraken extension install/update behaviorclassDiagram
class ExtensionSettings {
+str identifier
+str name
+str docker
+str tag
+Any permissions
+bool enabled
+Any user_permissions
}
class Extension {
+str identifier
+str tag
+Any source
+Any unique_entry
+Any lock(key)
+Any unlock(key)
+async _disable_running_extension() Optional_Extension
+_create_extension_settings(should_enable bool) ExtensionSettings
+async install(clear_remaining_tags bool, atomic bool, should_enable bool) AsyncGenerator_bytes_None
+async update(clear_remaining_tags bool, should_enable bool) AsyncGenerator_bytes_None
+async uninstall() None
+async _clear_remaining_tags() None
}
class ExtensionRouterV2 {
+async update_to_tag(identifier str, tag str, purge bool, should_enable bool) Response
}
ExtensionSettings <.. Extension : creates
Extension <.. ExtensionRouterV2 : used_by
class KrakenManager {
+installExtension(extension InstalledExtensionData, progressHandler Function, signal AbortSignal) Promise_void
+updateExtensionToVersion(identifier str, version str, progressHandler Function, signal AbortSignal) Promise_void
+finalizeExtension(extension InstalledExtensionData, tempTag str, progressHandler Function, signal AbortSignal) Promise_void
+uninstallExtension(identifier str) Promise_void
+uninstallExtensionVersion(identifier str, tag str) Promise_void
}
class ExtensionManagerView {
+AbortController active_abort_controller
+beginInstallOperation() AbortController
+cancelInstallOperation() void
+finishInstallOperation() void
+showAlertError(error any) void
+swapExtensionVersion(identifier str, enableTag str, removeTag str) Promise_void
+getTracker(signal AbortSignal) PullTracker
}
ExtensionManagerView --> KrakenManager : calls
ExtensionRouterV2 --> Extension : streams_to
KrakenManager --> ExtensionRouterV2 : HTTP_calls
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The new
uninstallExtensionVersionhelper callsDELETE /extension/${identifier}/${tag}, but there is no corresponding v2 route added inextension.py; if this endpoint doesn’t already exist elsewhere, this will consistently 404 and should either be implemented or the URL adjusted to an existing route. - In
finalizeExtensionUpload, theaxios.isCancelbranch always resetsinstall_from_file_phaseto'ready'and clearsinstall_from_file_status_textbefore checking whethercontroller === this.active_abort_controller; this can cause a stale controller’s cancellation to reset the UI during a new operation, so consider moving the UI reset inside thecontroller === this.active_abort_controllercheck. - The
PullProgressdialog is always rendered ascancelablefromExtensionManagerVieweven when there is no active abortable operation; tyingcancelableto!!active_abort_controllerwould prevent showing a cancel button that can’t actually affect any in-flight request.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `uninstallExtensionVersion` helper calls `DELETE /extension/${identifier}/${tag}`, but there is no corresponding v2 route added in `extension.py`; if this endpoint doesn’t already exist elsewhere, this will consistently 404 and should either be implemented or the URL adjusted to an existing route.
- In `finalizeExtensionUpload`, the `axios.isCancel` branch always resets `install_from_file_phase` to `'ready'` and clears `install_from_file_status_text` before checking whether `controller === this.active_abort_controller`; this can cause a stale controller’s cancellation to reset the UI during a new operation, so consider moving the UI reset inside the `controller === this.active_abort_controller` check.
- The `PullProgress` dialog is always rendered as `cancelable` from `ExtensionManagerView` even when there is no active abortable operation; tying `cancelable` to `!!active_abort_controller` would prevent showing a cancel button that can’t actually affect any in-flight request.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
8b9d9f7 to
453b3c1
Compare
There was a problem hiding this comment.
There is one edge case that seem to be happening with the PR that is the following:
- Have extension A V1 currently installed
- Go to extension store and open extension A popup
- Select extension A V2 and start the update
- Before the update download be complete, cancel it using the new
cancelbutton - Check Kraken settings file (Both extension A V1 and V2 are gone)
- Check docker images
docker image ls -a, extension A image is left there
Seems that this behavior breaks the atomic behavior of the Kraken install operation and could lead to images leaking on the docker side. Since we can't just delete images that are not associated with extensions given that it could be a user custom image used to other function.
453b3c1 to
9e04f46
Compare
|
@nicoschmdt can you take a look in the comments and CI ? |
fix: #3631
since we intend to move away from FastAPI and into zenoh I focused the operation cancellation handling mostly on the frontend code.
Summary by Sourcery
Add frontend and backend support for cancelling ongoing extension install and update operations, including proper rollback and UI feedback.
New Features:
Bug Fixes:
Enhancements: