kraken: allow cancelling install/update operations#3857
Open
nicoschmdt wants to merge 2 commits intobluerobotics:masterfrom
Open
kraken: allow cancelling install/update operations#3857nicoschmdt wants to merge 2 commits intobluerobotics:masterfrom
nicoschmdt 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
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
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.
nicoschmdt
force-pushed
the
stop-install
branch
from
8b9d9f7 to
453b3c1
Compare
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
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: