search

opendatahub-io / kubeflow

Machine Learning Toolkit for Kubernetes
Apache License 2.0
10 stars 35 forks source link

detail improvement odh notebook controller proxy context #344

Open shalberd opened 5 months ago

shalberd commented 5 months ago

/kind feature

Why you need this feature: code style improvement compared to https://github.com/opendatahub-io/kubeflow/pull/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 https://github.com/opendatahub-io/kubeflow/blob/7401baff76e07f87b75a9ae4bdbb69431144722f/components/odh-notebook-controller/controllers/notebook_webhook.go#L251

https://github.com/opendatahub-io/kubeflow/blob/7401baff76e07f87b75a9ae4bdbb69431144722f/components/odh-notebook-controller/controllers/notebook_webhook.go#L229

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 commented 5 months ago

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