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
feat: java11 request options timeout. #1233
Conversation
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.
Change looks good, but, can we have some testing as well?
java11 javadoc:
But I found that it is not very good in the actual test, and running the test directly does not work, but I broke the point to the Windows 10 and amazon-corretto-11.0.7.10.1-windows-x64. |
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.
LGTM, can merge once the build is passing.
ok! |
@ChunMengLu , @velo : One thing i noticed is, that you only considered the request timeout, but not the connect timeout. This would have to be set on the HttpClient instance (e.g. via Builder::connectTimeout). One could do this manually by providing a custom built HttpClient to Http2Client. However it would be good to leave this over to Http2Client when it constructs the HttpClient for us. E.g. by replacing the default constructor with a constructor taking Options or (IMHO better) lazily instantiating HttpClient - if no custom was specified via constructor - when the request should be sent. |
Config feign-java11 request timeout use feign
Request.options
.