feat: support namespaceFilter on GET /workspacekinds#826
feat: support namespaceFilter on GET /workspacekinds#826utruong309 wants to merge 3 commits intokubeflow:notebooks-v2from
Conversation
Signed-off-by: utruong309 <uyenthutruong09@gmail.com>
andyatmiami
left a comment
There was a problem hiding this comment.
@utruong309 - really appreciate you becoming an active participant in the community by raising this PR!
I am nit-picking a lot here on your submission - but please know overall I am very happy with your grasp of this domain. please let me know if you have any questions on the comments/requests for change(s) I have made.
hopefully none of these asks are controversial and you are able to get them in fairly quickly - as I think we can then move to get this PR merged shortly thereafter 💯
|
[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 |
andyatmiami
left a comment
There was a problem hiding this comment.
thanks for prompt resolution of initial round of comments.
i took a little deeper/more focused look at the current state of PR - and found a few other areas we will want to address. sorry I had overlooked these in initial review!
we are getting closer - please bear with me as we harden this! These should (hopefully) be the last of the significant changes I would request.
Signed-off-by: utruong309 <uyenthutruong09@gmail.com>
|
Hey @andyatmiami, Thanks for the detailed feedback! I've addressed all the comments. I also ran tests locally and everything looks good. Please let me know if there’s anything else you’d like me to improve! |
|
/ok-to-test |
andyatmiami
left a comment
There was a problem hiding this comment.
Just a heads up - as I was trying to (thoroughly) test various scenarios in your PR - particularly around setting DISABLE_AUTH: false on the backend deployment in our tilt cluster - I found a couple issues with our current existing implementation that is outside the scope of the work you are doing.
I am going to look to try and open up a fix ASAP to get it reviewed and merged.. but I will want to wait for that code to land.. so that then you can rebase this PR on top of that code and I can continue then testing to see your code working with SubjectAccessReview in our auth layer actually working
Stay tuned!
Summary
Adds optional
namespaceFilterquery parameter toGET /workspacekindsto support Bella-centric listing while preserving existing admin behavior.Behavior
namespaceFilteris provided:CREATE Workspaceauthorization in the given namespaceLIST WorkspaceKindauthorizationMotivation
This supports the dual-persona nature of
/workspacekinds:Acceptance Criteria