feat: remove automatic redirect application from Workspace controller#804
feat: remove automatic redirect application from Workspace controller#804alokdangre wants to merge 6 commits intokubeflow:notebooks-v2from
Conversation
This commit removes the automatic redirect application logic from the Workspace controller and shifts to user-initiated configuration updates. Changes: - Remove auto-update block (lines 218-237) from workspace_controller.go that automatically applied redirects when workspace was paused - Remove DeferUpdates field from WorkspaceSpec CRD - Remove DeferUpdates from backend API models (WorkspaceCreate, Workspace) - Update documentation in sample WorkspaceKind YAML to reflect that redirects are now user-initiated via API - Update all test fixtures to remove DeferUpdates field - Regenerate CRD manifests The controller still computes redirect chains and populates status fields: - status.podTemplateOptions.imageConfig.desired - status.podTemplateOptions.imageConfig.redirectChain - status.podTemplateOptions.podConfig.desired - status.podTemplateOptions.podConfig.redirectChain - status.pendingRestart Users must now explicitly update their Workspace via the PUT API to apply configuration redirects, providing better control and audit trail. Signed-off-by: alokdangre <alokdangre@gmail.com>
Remove all references to deferUpdates from the frontend codebase: - Remove deferUpdates from WorkspaceFormProperties type - Remove deferUpdates checkbox from workspace form UI - Remove deferUpdates from workspace creation payload - Remove deferUpdates from all mock data and test fixtures - Update OpenAPI swagger.json to remove deferUpdates field - Regenerate frontend API types from updated swagger spec - Add missing axe-core dependency for eslint - Remove unused Checkbox import The frontend now correctly reflects that redirect updates must be user-initiated via the API rather than automatically applied. Signed-off-by: alokdangre <alokdangre@gmail.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
google-oss-prow
bot
added
area/controller
Update swagger.version to point to the commit with updated swagger.json that properly removes deferUpdates field from the API types. Signed-off-by: alokdangre <alokdangre@gmail.com>
Screen.Recording.2025-12-14.202605.mp4 |
|
/ok-to-test |
caponetto
left a comment
caponetto
left a comment
There was a problem hiding this comment.
Thanks for your contribution @alokdangre!
I reviewed the frontend changes and left a comment inline.
| @@ -1 +1 @@ | |||
| 4f0a29dec0d3c9f0d0f02caab4dc84101bfef8b0 | |||
| 4e1c149 | |||
There was a problem hiding this comment.
Could you please use the latest commit hash we have in the repository for the swagger.json file?
It's b81728498cbd567be42482f941a97ac551f11253 (see here)
| 4e1c149 | |
| b81728498cbd567be42482f941a97ac551f11253 |
There was a problem hiding this comment.
I see the change I proposed here got reverted in the last commits
There was a problem hiding this comment.
b81728498cbd567be42482f941a97ac551f11253 this has deferUpdates in it , so when the ci generates types it fails becuase we don't use deferUpdates, so therefore i changed it to the f94d911fac3ec4f4f231fff926a4c7cd49c67b3c this commit where the swagger has no defeUpdates, so ci generated type check will pass
There was a problem hiding this comment.
Yeah, I see!
@andyatmiami that's why we generally have 2 PRs (frontend/backend) when there's update in the API.
There was a problem hiding this comment.
sorry, i added both at one because ,by adding backend only changes, the frontend workspace creation was broked thats why i added both at one
There was a problem hiding this comment.
No worries @alokdangre! Appreciate the proactivity! I think this is the first time we have faced this scenario.
My only concern about having a single PR is the need to reference the hash commit from one that is not technically present in this repository. I'll let @andyatmiami weigh in on that decision.
Co-authored-by: Guilherme Caponetto <638737+caponetto@users.noreply.github.com> Signed-off-by: Alok Dangre <148090007+alokdangre@users.noreply.github.com>
- Regenerate swagger docs (docs.go) to remove deferUpdates - Remove deferUpdates from sample workspace YAML - Update test comments to remove deferUpdates references - Update backend README example to remove deferUpdates - Update frontend package-lock.json Signed-off-by: alokdangre <alokdangre@gmail.com>
alokdangre
force-pushed
the
feat/remove-workspace-auto-redirect
branch
2 times, most recently
from
9450c94 to
896170d
Compare
alokdangre
force-pushed
the
feat/remove-workspace-auto-redirect
branch
3 times, most recently
from
46aabbc to
b546654
Compare
- Update swagger.version to point to current commit without deferUpdates - Regenerate frontend API types from updated swagger.json - Update package-lock.json Signed-off-by: alokdangre <alokdangre@gmail.com>
|
so as decieded i have made a separate backend only pr #812 |
|
and i have also made a frontend only changes pr: #813 |
|
Thank you @alokdangre! In that case, this PR could probably be closed. |
github-project-automation
bot
moved this from Needs Triage
to Done
in Kubeflow Notebooks
3 participants
This PR removes the automatic application of redirects by the Workspace controller and transitions to a user-initiated update model via the API.
closes: #779
Changes:
DeferUpdatesfield fromWorkspaceSpecin CRD and backend models.workspace_controller.gothat automatically mutatedWorkspace.Specbased on redirects.DeferUpdates.DeferUpdatescheckbox and logic.This ensures that workspace updates (redirects) are only applied when explicitly requested by the user, providing a safer and more predictable update process.