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

Repair FeignClient: Use feign dependencies for okhttp/httpclient #42

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

tfabien
Copy link

@tfabien tfabien commented Mar 16, 2021

Use io.github.openfeign:feign-okhttp and io.github.openfeign:feign-httpclient instead of com.squareup.okhttp3:okhttp and org.apache.httpcomponents:httpclient
If not, Feign client won't use these clients and won't honor the feign.okhttp.enabled / feign.httpclient.enabled configuration (and fallback to sun httpClient, without using the proxy selector)

tfabien added 2 commits March 16, 2021 18:40
Use `io.github.openfeign:feign-okhttp` and `io.github.openfeign:feign-httpclient instead` of `com.squareup.okhttp3:okhttp` and `org.apache.httpcomponents:httpclient`
If not, Feign client won't use these clients and won't honor the `feign.okhttp.enabled` / `feign.httpclient.enabled` configuration (and fallback to sun httpClient, without using the proxy selector)
Repair FeignClient: Use feign dependencies for okhttp/httpclient
@deveth0
Copy link
Owner

deveth0 commented Mar 16, 2021

hey, thanks for the PR!

Are you sure, those changes are sufficient, they only change the sample-project but not the starter / autoconfig:
https://github.com/deveth0/httpclient-spring-boot-starter/blob/develop/httpclient-spring-boot-autoconfigure/pom.xml
https://github.com/deveth0/httpclient-spring-boot-starter/blob/develop/httpclient-spring-boot-starter/pom.xml

@tfabien
Copy link
Author

tfabien commented Mar 16, 2021

I'll update the httpclient-spring-boot-autoconfigure/pom.xml, keeping them optional
For httpclient-spring-boot-starter/pom.xml however, only httpclient is included at the moment, and it's not optional, should I include okhttpand make both optional?
Furthermore, shouldn't these dependencies only be declare in httpclient-spring-boot-autoconfigure ? (not familiar with the optional parameter, I don't know if I need to repeat these dependencies)

@deveth0
Copy link
Owner

deveth0 commented Mar 16, 2021

interesting question, i'm 100% sure, if it's required to have those in starter, but given that the official starters also include depencendies (and the autoconfigs optional dependencies), I guess we should keep em ;)
https://github.com/spring-projects/spring-boot/blob/master/spring-boot-project/spring-boot-starters/spring-boot-starter-data-couchbase/build.gradle

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 this pull request may close these issues.

2 participants