-
Notifications
You must be signed in to change notification settings - Fork 815
Allow Customizer for HttpClientConnectionManager in HttpClient5FeignConfiguration #1305
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
Allow Customizer for HttpClientConnectionManager in HttpClient5FeignConfiguration #1305
Conversation
…gnConfiguration' via new customizer supplied as beans Signed-off-by: Severin Kistler <kistlerseverin@gmail.com>
|
Could you add some tests and documentation for this feature? |
Signed-off-by: Severin Kistler <kistlerseverin@gmail.com>
fb7b44f to
48be6e1
Compare
…BuilderCustomizer, as it already exists for httpClient5FeignConfiguration.HttpClientBuilderCustomizer a few lines above Signed-off-by: Severin Kistler <kistlerseverin@gmail.com>
2673c00 to
c23f06e
Compare
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.
Pull request overview
This PR adds support for customizing the HttpClientConnectionManager in HttpClient5FeignConfiguration through a new HttpClientConnectionManagerBuilderCustomizer interface, mirroring the existing customization pattern for CloseableHttpClient. This addresses the need to configure connection manager settings like TLS handshake timeouts without overriding the entire bean configuration.
Key Changes:
- Introduced
HttpClientConnectionManagerBuilderCustomizerinterface for programmatic connection manager configuration - Updated
hc5ConnectionManagerbean to accept and apply customizers - Added documentation and test coverage for the new customization mechanism
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| HttpClient5FeignConfiguration.java | Added new HttpClientConnectionManagerBuilderCustomizer interface and updated hc5ConnectionManager to apply customizers |
| FeignHttpClient5ConfigurationTests.java | Added test coverage for the new connection manager customizer functionality |
| spring-cloud-openfeign.adoc | Updated documentation with examples showing both HTTP client and connection manager customization |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...ain/java/org/springframework/cloud/openfeign/clientconfig/HttpClient5FeignConfiguration.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Ryan Baxter <ryan.baxter@broadcom.com>
…cloud/openfeign/clientconfig/HttpClient5FeignConfiguration.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Ryan Baxter <ryan.baxter@broadcom.com>
Is your feature request related to a problem? Please describe.
The underlying http client (in my case the apache hc5) is already well-customizable via properties and also via supplying my own beans. I'd like a further middle ground by allowing a list of customzer for
HttpClientConnectionManagerinHttpClient5FeignConfiguration, as it also exists for theCloseableHttpClientin the same file.In my case, I need to configure the
TlsConfig::handshakeTimeoutas a result of the new handling in 5.5.1 (see https://issues.apache.org/jira/browse/HTTPCLIENT-2386)Describe the solution you'd like
Allow a list of
ObjectProvider<List<HttpClientConnectionManagerBuilderCustomizer>> customizerProviderto thehc5ConnectionManagerbean method.Describe alternatives you've considered
Yes, I know I can override the full
HttpClientConnectionManageras my own bean, but then I loose the rest of the auto-configuration logic for this bean. I can also not just copy the bean metho and add my own logic, as I would also have to copy private methods likehttpsSSLConnectionSocketFactoryand my implementation would, over time, surely diverge for the curated defaults that I would like to keep and profit from.Additional context
I am happy to create a PR, unless there is a good reason why this feature does and should not exist.
This PR implements #1304
Edit: copied over issue description from #1304