-
Notifications
You must be signed in to change notification settings - Fork 195
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
frontend: Introduce kind, apiVersion, apiName fields for all KubeObjects #2399
Conversation
frontend/src/lib/k8s/clusterRole.ts
Outdated
static kind = 'ClusterRole'; | ||
static apiName = 'clusterroles'; | ||
static apiVersion = 'rbac.authorization.k8s.io/v1'; | ||
static isNamespaced = false; | ||
|
||
static apiEndpoint = apiFactory('rbac.authorization.k8s.io', 'v1', 'clusterroles'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This means we are repeating the apiEndpoint's args with the apiVersion and isNamespaced vars.
Would it work and be backward compat if we had apiEndpoint be a getter by default? So we could return apiFactory/WithNamespace using the apiVersion and isNamespaced vars.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
apiEndpoint still contains methods like get,post,put that are still used, so we can't replace it with a getter yet. I think this duplication is okay for now, after v2 migration we can get rid of apiEndpoint completely
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My point is that we need the apiEndpoint for now but we can generate it ourselves by having a getter. Let me try adding a fixup commit.
So we can get rid of This keeps all the same values (although casing will be consistent) and gets rid of all the other overrides. ResourceClasses keys are manually converted to title case, so Before (objectName, objectName+s and overrides)
After (kind and apiName)
|
289b29a
to
94dcb6b
Compare
Signed-off-by: Oleksandr Dubenko <oldubenko@microsoft.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this refactor! cleans things up well
…formation Signed-off-by: Oleksandr Dubenko <oldubenko@microsoft.com>
this change will be part of #1967 PR |
Fixes #2384