-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Fixes #11031 - HttpClient should expose Connection/EndPoint used by H… #11033
Conversation
…TTP requests. Introduced method Request.getConnection() to expose the Connection after at the request begin event. Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
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 except that I'm not convinced by the default
marker of the new interface methods.
* or {@code null} if there is no connection associated | ||
* with this request | ||
*/ | ||
default Connection getConnection() |
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.
Why mark that method as default
?
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.
Because it is an existing interface that people may have implemented (e.g. for wrapping), and we don't want to break them.
/** | ||
* @return the local socket address associated with the connection | ||
*/ | ||
default SocketAddress getLocalSocketAddress() |
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.
Why mark that method as default
?
/** | ||
* @return the remote socket address associated with the connection | ||
*/ | ||
default SocketAddress getRemoteSocketAddress() |
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.
Why mark that method as default
?
…TTP requests.
Introduced method Request.getConnection() to expose the Connection after at the request begin event.