-
Notifications
You must be signed in to change notification settings - Fork 134
New transport: apache5x #742
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
base: master
Are you sure you want to change the base?
Conversation
That is using ASF httpclient5x (5.5 currently).
Current status:
|
This also raises following question about Maven core:
|
@michael-o can you take a peek please? Also, I copied "apache" transport that was written in very specific way (ie globally shared state etc), that may not be needed (or will not work anymore) for httpclient 5.5.x... |
Will do tomorrow |
Migrating to 5.5.x "classic" API is IMO completely enough, unless you think we should do something else. |
<dependency> | ||
<groupId>org.apache.httpcomponents.core5</groupId> | ||
<artifactId>httpcore5-h2</artifactId> | ||
<version>5.3.4</version> |
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.
Use a property
|
||
<artifactId>maven-resolver-transport-apache5x</artifactId> | ||
|
||
<name>Maven Artifact Resolver Transport Apache 5.x</name> |
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.
That is a bad name. Apache is, unfortunately, used synonymously for Apache HTTPd. Use the full name.
@@ -28,7 +28,7 @@ | |||
|
|||
<artifactId>maven-resolver-transport-apache</artifactId> | |||
|
|||
<name>Maven Artifact Resolver Transport Apache</name> | |||
<name>Maven Artifact Resolver Transport Apache 4.x</name> |
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.
Same here
<version>2.0.10-SNAPSHOT</version> | ||
</parent> | ||
|
||
<artifactId>maven-resolver-transport-apache5x</artifactId> |
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.
apache-httpclient5
@@ -120,6 +120,13 @@ To modify this file, edit the template and regenerate. | |||
| `"aether.transport.apache.retryHandler.name"` | `String` | The name of retryHandler, supported values are “standard”, that obeys RFC-2616, regarding idempotent methods, and “default” that considers requests w/o payload as idempotent. | `"standard"` | 2.0.0 | Yes | Session Configuration | | |||
| `"aether.transport.apache.retryHandler.requestSentEnabled"` | `Boolean` | Set to true if it is acceptable to retry non-idempotent requests, that have been sent. | `false` | 2.0.0 | Yes | Session Configuration | | |||
| `"aether.transport.apache.useSystemProperties"` | `Boolean` | If enabled, underlying Apache HttpClient will use system properties as well to configure itself (typically used to set up HTTP Proxy via Java system properties). See HttpClientBuilder for used properties. This mode is not recommended, better use documented ways of configuration instead. | `false` | 2.0.0 | Yes | Session Configuration | | |||
| `"aether.transport.apache5x.followRedirects"` | `Boolean` | If enabled, Apache HttpClient will follow HTTP redirects. | `true` | 2.0.10 | Yes | Session Configuration | |
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 x actually? Isn't 5 enough?
|
||
|
||
<site xmlns="http://maven.apache.org/SITE/2.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" | ||
xsi:schemaLocation="http://maven.apache.org/SITE/2.0.0 https://maven.apache.org/xsd/site-2.0.0.xsd" name="Transport Apache"> |
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.
Ditto for name
} | ||
return false; | ||
} | ||
} |
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.
The client package or even core might include UriUtils. Did you check?
} | ||
} | ||
|
||
public Boolean getWebDav() { |
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 boolean object?
*/ | ||
@Named(ApacheTransporterFactory.NAME) | ||
public final class ApacheTransporterFactory implements HttpTransporterFactory { | ||
public static final String NAME = "apache5x"; |
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.
ditto
@arturobernalg @ok2c @garydgregory: Please have a look. |
@michael-o Sure. I will do a pass |
This was a misunderstanding. This PR is not "done-done". I asked @michael-o to "take over", if possible. |
Unfortuntely, my exprience with 5.x is basically zero. I do even maintain 4.x based client code at work. I'll be of very little help here. |
That is using ASF httpclient5x (5.5 currently). Took existing "apache" transport and migrated it to "classic" of 5.5 for now. Tests are not passing as I probably introduced some migration issues (or just overlooked something).
WIP