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

KubernetesClientBuilder enhancements for common HttpClient customizations #4471

Closed
shawkins opened this issue Sep 30, 2022 · 0 comments · Fixed by #4490
Closed

KubernetesClientBuilder enhancements for common HttpClient customizations #4471

shawkins opened this issue Sep 30, 2022 · 0 comments · Fixed by #4490
Assignees
Milestone

Comments

@shawkins
Copy link
Contributor

shawkins commented Sep 30, 2022

    > First of all, I should not have to create Kubernetes client first so that I can get http client builder to replace interceptor and then get http client factory and finally create Kubernetes clients from builder using that http client factory. This is pretty messy builder pattern implementation from API point of view.

That's obviously not intended. There are really two issues here. The first is that there's no current way exposed for getting the default HttpClient Factory. A method for that can easily be extracted from HttpClientUtils.createHttpClient. The second is that the factory logic assumes customization via sub-classing (see the XXXClientFactory.additionalConfig methods). There wasn't a mechanism envisioned for modifying across any factory - such as modifying the interceptors in a common way.

Why not pass HttpClientBuilder or HttpClientFactoryBuilder to KubernetesClientBuilder?

Since we added HttpClient.getFactory, HttpClient.Builder is viable as a possible argument to KubernetesClientBuilder. To fully decompose it's also possible to change HttpClient.Factory.createHttpClient(Config config) to be HttpClient.Factory.processConfig(Config config, HttpClient.Builder).

That looks approximately like:

HttpClient.Factory factory = HttpClient.Factory.new();
HttpClient.Builder builder = factory.newBuilder();
// make modifications that could be overridden by processing the config
factory.processConfig(config, builder);
// make modifications that override the config
builder.addOrReplaceInterceptor ...
// then supply the builder to the KubernetesClientBuilder

However it's also possible to just associate a Consumer<HttpClient.Builder> with the KubernetesClientBuilder:

new KubernetesClientBuilder()
  .withConfig(config)
  .withAdditionalHttpClientConfiguration(b -> b.addOrReplaceInterceptor... )
  .build();

That eliminates a few of the steps.

Originally posted by @shawkins in #2271 (comment)

@shawkins shawkins added this to the 6.2.0 milestone Sep 30, 2022
@shawkins shawkins self-assigned this Oct 11, 2022
shawkins added a commit to shawkins/kubernetes-client that referenced this issue Oct 12, 2022
shawkins added a commit to shawkins/kubernetes-client that referenced this issue Oct 12, 2022
shawkins added a commit to shawkins/kubernetes-client that referenced this issue Oct 13, 2022
shawkins added a commit to shawkins/kubernetes-client that referenced this issue Oct 17, 2022
manusa pushed a commit to shawkins/kubernetes-client that referenced this issue Oct 18, 2022
manusa pushed a commit to shawkins/kubernetes-client that referenced this issue Oct 18, 2022
manusa pushed a commit to shawkins/kubernetes-client that referenced this issue Oct 18, 2022
manusa pushed a commit to shawkins/kubernetes-client that referenced this issue Oct 18, 2022
@manusa manusa closed this as completed in 366ede8 Oct 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant