Skip to content
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

detail improvement odh notebook controller proxy context #344

Open
shalberd opened this issue Jun 11, 2024 · 1 comment
Open

detail improvement odh notebook controller proxy context #344

shalberd opened this issue Jun 11, 2024 · 1 comment
Labels
kind/feature New feature

Comments

@shalberd
Copy link

/kind feature

Why you need this feature:
code style improvement compared to #326

and

issue #291

Thank you @jiridanek

@harshad16 fyi

Describe the solution you'd like:
use context from NotebookWebHook and pass it in as context instead of TODO

func (w *NotebookWebhook) Handle(ctx context.Context, req admission.Request) admission.Response {

err := w.Client.List(context.TODO(), proxyResourceList)

proxyEnvVars does not need to be a global variable, especially since every call to ClusterWideProxyIsEnabled creates its content anew. Can we return the proxy env vars from the function?

https://github.com/opendatahub-io/kubeflow/pull/326/files/7401baff76e07f87b75a9ae4bdbb69431144722f#diff-1df80e242be90c94fce3ffb051a0bfe706fe924dab62438c7aa0a186b8067153R227

Anything else you would like to add:
[Miscellaneous information that will assist in solving the issue.]

@jiridanek
Copy link
Member

when we have go 1.21 available (release notes https://go.dev/blog/go1.21), there's opportunities to use https://pkg.go.dev/slices package

see the blog about it at https://go.dev/blog/generic-slice-functions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature New feature
Projects
None yet
Development

No branches or pull requests

2 participants