-
Notifications
You must be signed in to change notification settings - Fork 584
Enhance Apache HTTP client #1171
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
Conversation
@zhicwu I will test it tomorrow |
*/ | ||
SOCKET_REUSEADDR("socket_reuseaddr", false, | ||
"Whether allows for the reuse of local addresses and ports. " | ||
+ "Only works for client using custom Socket(e.g. TCP client or HTTP provider with custom SocketFactory etc.)."), |
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.
@zhicwu does custom SocketFactory refers to ApacheHttpConnectionImpl.SocketFactory
and ApacheHttpConnectionImpl.SSLSocketFactory
?
I am wondering if it is possible to inject a custom socket factory.
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.
Does custom SocketFactory refers to ApacheHttpConnectionImpl.SocketFactory and ApacheHttpConnectionImpl.SSLSocketFactory?
Yes, for now. Ideally we should have a custom socket factory for not only Apache HTTP Client but also others(i.e. NIO-based implementation).
I am wondering if it is possible to inject a custom socket factory.
Sorry it's not supported at this point. What's the purpose of injecting custom socket factory? Is it to use consistent socket options across datasources?
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.
Thanks for confirming. Yes, I would like to create a custom socket.
Are you planning to include that feature in the future? Other JDBC drivers(eg. mysql) offer such capabilities.
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.
Thanks for the suggestion. Just created a separate issue for this, should not be hard to add the feature.
This is mainly a follow-up PR for #1146 with below changes:
socket_op_ip_tos
tosocket_ip_tos
for consistency @JackyWooByteArraysInputStream
because it's been covered by ClickHouseInputStream