-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Minor simplification in load balancer supplier #1579
Conversation
Configure Renovate
this.kubernetesNamespaceProvider = kubernetesNamespaceProvider; | ||
} | ||
|
||
private String getNamespace() { |
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 one is a little tricky, but not much.
we used to have it as:
return kubernetesNamespaceProvider != null ? kubernetesNamespaceProvider.getNamespace()
: kubernetesClientProperties.namespace();
let's asume kubernetesNamespaceProvider == null
, which means kubernetesClientProperties.namespace()
will be called. But there is no code that would provide kubernetesClientProperties
, which means it was always null
.
So if ever kubernetesNamespaceProvider == null
, we would have got a NPE from kubernetesClientProperties.namespace()
. So let's get rid of KubernetesClientProperties kubernetesClientProperties
and only use KubernetesNamespaceProvider
.
But even by doing that, we still keep the initial "intention", which I assume was that the KubernetesNamespaceProvider
at the time of writing the code, did not have support for spring.cloud.kubernetes.client.namespace
reading, that is why it was reading it from KubernetesClientProperties
. Now this supports exists there, so I think the code works now as the initial intention.
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.
Yeah that is very weird, makes sense.
@ryanjbaxter ready to be looked at. thank you. |
No description provided.