-
Notifications
You must be signed in to change notification settings - Fork 353
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
Apache httpclient 5 #4950
Apache httpclient 5 #4950
Conversation
Thanks for the new PR. |
@jansupol Then I'll modify this PR to use a seperate module for the new apache-connector and remove the other updates to HttpClient 5.x to prevent any dependency conflicts. If I may ask this: I'm relatively new to OSS contributions and don't know exactly how to get the licenses and copyrights correct. |
@zUniQueX regarding copyrights - for new files (creating new module and copying files from another module into that module basically means that all files in that module are new) it's required to keep license as
with only one year (2022) as a creation year in the first sentence. The 2nd year in the copyright sentence appears when a file is modified during another year than the creation year. And when PR is submitted there is a copyright check during the related build which would fail the build if there is no license/copyright or any of indicated years (creation/modification) is wrong. So, if the build passes OK this means everything (including copyright) is correct. That's basically all regarding CP/license in created/modified files. And thank you for the contribution, it's appreciated. |
@senivam Thanks for the explanation. |
Copy the existing Apache connector to a new module and migrate it to HttpClient 5.x Classic APIs Signed-off-by: Steffen Nießing <zuniquex@protonmail.com>
5818462
to
8c9193f
Compare
@jansupol I've modified this PR to implement the changes according to your suggestion. Therefore I moved the files to a separate module, modified the copyright years while keeping the JavaDoc authors, modified the client properties to use a separate namespace and removed some unused code, which was left over for earlier HttpClient 4.x support. |
I just looked for an easy way maintaining both connector implementations and tests together, but haven't found a nice solution. @jansupol Should we deprecate the Apache 4.x connector to remove it in 3.1.0? Or should we maintain both versions concurrently? |
@zUniQueX First of all I want to say that this PR looks good. I am reviewing it in more detail, but so far it looks good. We should not remove the 4.x connector in 3.1, maybe we can do it in 4.0 (Jakarta EE 11). We would need to maintain both. |
Okay, this sounds reasonable to me. So far, I just performed the migration steps and didn't look further into HTTP/2 support. But I understand, that this should be supported in a later version. |
Should we now backport #4969 to this? Or do other things on the existing Apache connector have to be changed prior to merging this? |
Port fix from eclipse-ee4j#4969 to HttpClient 5.x Signed-off-by: Steffen Nießing <zuniquex@protonmail.com>
Sorry, it took a while to get back to this. I hope I wasn't blocking the release of 2.36. |
@zUniQueX We are sorry it takes so long to review - we definitely plan to have this in 2.36 - I assume around the end of April. |
@jansupol Thanks for the fast answer. I'm personally not waiting for this to release, I just wanted to make sure I'm not blocking your release as this is listed in the Wiki for 2.36. |
...apache5-connector/src/main/java/org/glassfish/jersey/apache5/connector/Apache5Connector.java
Outdated
Show resolved
Hide resolved
Remove exception on null content type Signed-off-by: Steffen Nießing <zuniquex@protonmail.com>
CQ is resolved, merging... |
Migrate the Apache HTTP Client to version 5.x. The migration guide provides several different migration steps, but this PR only migrates to the classic APIs, since the async APIs have less support for
InputStream / OutputStream
based content processing.Notable changes:
HttpRequestRetryHandler
was replaced by the newHttpRequestRetryStrategy
. Therefore the client propertyApache5ClientProperties#RETRY_HANDLER
is renamed toApache5ClientProperties#RETRY_STRATEGY
CredentialsProvider
now only allows to retrieve credentials. For setting credentials, the superinterfaceCredentialsStore
has to be used