You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "cstamas (via GitHub)" <gi...@apache.org> on 2023/03/07 11:03:26 UTC
[GitHub] [maven-resolver] cstamas opened a new pull request, #266: [MRESOLVER-327] Allow use of system properties
cstamas opened a new pull request, #266:
URL: https://github.com/apache/maven-resolver/pull/266
But do not encourage users about this, quite the opposite, drive them toward documentation how to configure transports, for example this page: https://maven.apache.org/guides/mini/guide-proxies.html
---
https://issues.apache.org/jira/browse/MRESOLVER-327
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-resolver] cstamas commented on a diff in pull request #266: [MRESOLVER-327] Allow use of system properties to configure HttpClient
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #266:
URL: https://github.com/apache/maven-resolver/pull/266#discussion_r1127761628
##########
src/site/markdown/configuration.md:
##########
@@ -41,6 +41,7 @@ Option | Type | Description | Default Value | Supports Repo ID Suffix
`aether.connector.http.preemptivePutAuth` | boolean | Should HTTP client use preemptive-authentication for HTTP PUTs only (works only w/ BASIC). By default is enabled (same as Wagon). | `true` | yes
`aether.connector.http.retryHandler.count` | int | The maximum number of times a request to a remote HTTP server should be retried in case of an error. | `3` | yes
`aether.connector.http.supportWebDav` | boolean | If enabled, transport makes best effort to deploy to WebDAV server. This mode is not recommended, better use real Maven Repository Manager instead. | `false` | yes
+`aether.connector.http.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). This mode is not recommended, better use proper Maven configuration means instead. | `false` | yes
Review Comment:
I consider having both set an error (and at one point I had code even fail), but later realized to simply "delegate" all the responsibility to user: "user knows what he does"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-resolver] cstamas commented on a diff in pull request #266: [MRESOLVER-327] Allow use of system properties to configure HttpClient
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #266:
URL: https://github.com/apache/maven-resolver/pull/266#discussion_r1127758035
##########
src/site/markdown/configuration.md:
##########
@@ -41,6 +41,7 @@ Option | Type | Description | Default Value | Supports Repo ID Suffix
`aether.connector.http.preemptivePutAuth` | boolean | Should HTTP client use preemptive-authentication for HTTP PUTs only (works only w/ BASIC). By default is enabled (same as Wagon). | `true` | yes
`aether.connector.http.retryHandler.count` | int | The maximum number of times a request to a remote HTTP server should be retried in case of an error. | `3` | yes
`aether.connector.http.supportWebDav` | boolean | If enabled, transport makes best effort to deploy to WebDAV server. This mode is not recommended, better use real Maven Repository Manager instead. | `false` | yes
+`aether.connector.http.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). This mode is not recommended, better use proper Maven configuration means instead. | `false` | yes
Review Comment:
Originally I wanted to WARN if `proxy != null` (set few lines above), but this builder option is NOT ONLY about proxy but many many other things. Hence the hint "typically used..." as I cannot nor I want to commit to any guarantees here. Simply put: if this option used, user should use it only for "typically set" stuff, and ensure the code is doing what he things is doing.
The reason of existence of this PR is ONLY to allow use of transport-http for use case like https://issues.apache.org/jira/browse/MNG-7721 where user defines only 5 http proxy related properties.
Given when this enabled, httpClient OBEY MANY system properties, and we CANNOT control which one it obeys, this is the cleanest. This PR will work for reporter of MNG-7721 but I have no idea for other use cases.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-resolver] cstamas commented on pull request #266: [MRESOLVER-327] Allow use of system properties to configure HttpClient
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #266:
URL: https://github.com/apache/maven-resolver/pull/266#issuecomment-1458230065
Am merging this as doco was updated, and is also safe as all this is disabled by default. The only reason for this change is to make possible exact use case like the reported in https://issues.apache.org/jira/browse/MNG-7721 to work with transport-http, nothing more and nothing less.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-resolver] cstamas merged pull request #266: [MRESOLVER-327] Allow use of system properties to configure HttpClient
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas merged PR #266:
URL: https://github.com/apache/maven-resolver/pull/266
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-resolver] kwin commented on a diff in pull request #266: [MRESOLVER-327] Allow use of system properties to configure HttpClient
Posted by "kwin (via GitHub)" <gi...@apache.org>.
kwin commented on code in PR #266:
URL: https://github.com/apache/maven-resolver/pull/266#discussion_r1127754033
##########
src/site/markdown/configuration.md:
##########
@@ -41,6 +41,7 @@ Option | Type | Description | Default Value | Supports Repo ID Suffix
`aether.connector.http.preemptivePutAuth` | boolean | Should HTTP client use preemptive-authentication for HTTP PUTs only (works only w/ BASIC). By default is enabled (same as Wagon). | `true` | yes
`aether.connector.http.retryHandler.count` | int | The maximum number of times a request to a remote HTTP server should be retried in case of an error. | `3` | yes
`aether.connector.http.supportWebDav` | boolean | If enabled, transport makes best effort to deploy to WebDAV server. This mode is not recommended, better use real Maven Repository Manager instead. | `false` | yes
+`aether.connector.http.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). This mode is not recommended, better use proper Maven configuration means instead. | `false` | yes
Review Comment:
I wouldn’t expect the term Maven configuration in here. Rather resolver configuration as resolver can still be used outside Maven
##########
src/site/markdown/configuration.md:
##########
@@ -41,6 +41,7 @@ Option | Type | Description | Default Value | Supports Repo ID Suffix
`aether.connector.http.preemptivePutAuth` | boolean | Should HTTP client use preemptive-authentication for HTTP PUTs only (works only w/ BASIC). By default is enabled (same as Wagon). | `true` | yes
`aether.connector.http.retryHandler.count` | int | The maximum number of times a request to a remote HTTP server should be retried in case of an error. | `3` | yes
`aether.connector.http.supportWebDav` | boolean | If enabled, transport makes best effort to deploy to WebDAV server. This mode is not recommended, better use real Maven Repository Manager instead. | `false` | yes
+`aether.connector.http.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). This mode is not recommended, better use proper Maven configuration means instead. | `false` | yes
Review Comment:
Can you state which one takes precedence in case both are set?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-resolver] cstamas commented on a diff in pull request #266: [MRESOLVER-327] Allow use of system properties to configure HttpClient
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #266:
URL: https://github.com/apache/maven-resolver/pull/266#discussion_r1127761628
##########
src/site/markdown/configuration.md:
##########
@@ -41,6 +41,7 @@ Option | Type | Description | Default Value | Supports Repo ID Suffix
`aether.connector.http.preemptivePutAuth` | boolean | Should HTTP client use preemptive-authentication for HTTP PUTs only (works only w/ BASIC). By default is enabled (same as Wagon). | `true` | yes
`aether.connector.http.retryHandler.count` | int | The maximum number of times a request to a remote HTTP server should be retried in case of an error. | `3` | yes
`aether.connector.http.supportWebDav` | boolean | If enabled, transport makes best effort to deploy to WebDAV server. This mode is not recommended, better use real Maven Repository Manager instead. | `false` | yes
+`aether.connector.http.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). This mode is not recommended, better use proper Maven configuration means instead. | `false` | yes
Review Comment:
I consider having both set an error (and at one point I had code even fail), but later realized to simple "delegate" all the responsibility to use: "user knows what he does"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-resolver] cstamas commented on a diff in pull request #266: [MRESOLVER-327] Allow use of system properties to configure HttpClient
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #266:
URL: https://github.com/apache/maven-resolver/pull/266#discussion_r1127791146
##########
src/site/markdown/configuration.md:
##########
@@ -41,6 +41,7 @@ Option | Type | Description | Default Value | Supports Repo ID Suffix
`aether.connector.http.preemptivePutAuth` | boolean | Should HTTP client use preemptive-authentication for HTTP PUTs only (works only w/ BASIC). By default is enabled (same as Wagon). | `true` | yes
`aether.connector.http.retryHandler.count` | int | The maximum number of times a request to a remote HTTP server should be retried in case of an error. | `3` | yes
`aether.connector.http.supportWebDav` | boolean | If enabled, transport makes best effort to deploy to WebDAV server. This mode is not recommended, better use real Maven Repository Manager instead. | `false` | yes
+`aether.connector.http.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). This mode is not recommended, better use proper Maven configuration means instead. | `false` | yes
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-resolver] michael-o commented on a diff in pull request #266: [MRESOLVER-327] Allow use of system properties to configure HttpClient
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #266:
URL: https://github.com/apache/maven-resolver/pull/266#discussion_r1127765717
##########
src/site/markdown/configuration.md:
##########
@@ -41,6 +41,7 @@ Option | Type | Description | Default Value | Supports Repo ID Suffix
`aether.connector.http.preemptivePutAuth` | boolean | Should HTTP client use preemptive-authentication for HTTP PUTs only (works only w/ BASIC). By default is enabled (same as Wagon). | `true` | yes
`aether.connector.http.retryHandler.count` | int | The maximum number of times a request to a remote HTTP server should be retried in case of an error. | `3` | yes
`aether.connector.http.supportWebDav` | boolean | If enabled, transport makes best effort to deploy to WebDAV server. This mode is not recommended, better use real Maven Repository Manager instead. | `false` | yes
+`aether.connector.http.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). This mode is not recommended, better use proper Maven configuration means instead. | `false` | yes
Review Comment:
The description must link to the constructor of HttpClientBuilder which will describe which properties from the system will be used.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-resolver] cstamas commented on a diff in pull request #266: [MRESOLVER-327] Allow use of system properties to configure HttpClient
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #266:
URL: https://github.com/apache/maven-resolver/pull/266#discussion_r1127758035
##########
src/site/markdown/configuration.md:
##########
@@ -41,6 +41,7 @@ Option | Type | Description | Default Value | Supports Repo ID Suffix
`aether.connector.http.preemptivePutAuth` | boolean | Should HTTP client use preemptive-authentication for HTTP PUTs only (works only w/ BASIC). By default is enabled (same as Wagon). | `true` | yes
`aether.connector.http.retryHandler.count` | int | The maximum number of times a request to a remote HTTP server should be retried in case of an error. | `3` | yes
`aether.connector.http.supportWebDav` | boolean | If enabled, transport makes best effort to deploy to WebDAV server. This mode is not recommended, better use real Maven Repository Manager instead. | `false` | yes
+`aether.connector.http.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). This mode is not recommended, better use proper Maven configuration means instead. | `false` | yes
Review Comment:
Originally I wanted to WARN if `proxy != null` (set few lines above), but this builder option is NOT ONLY about proxy but many many other things. Hence the hint "typically used..." as I cannot nor I want to commit to any guarantees here. Simply put: if this option used, user should use it only for "typically set" stuff, and ensure the code is doing what he thinks is doing.
The reason of existence of this PR is ONLY to allow use of transport-http for use case like https://issues.apache.org/jira/browse/MNG-7721 where user defines only 5 http proxy related properties.
Given when this enabled, httpClient OBEY MANY system properties, and we CANNOT control which one it obeys, this is the cleanest. This PR will work for reporter of MNG-7721 but I have no idea for other use cases.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-resolver] cstamas commented on a diff in pull request #266: [MRESOLVER-327] Allow use of system properties to configure HttpClient
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #266:
URL: https://github.com/apache/maven-resolver/pull/266#discussion_r1127758035
##########
src/site/markdown/configuration.md:
##########
@@ -41,6 +41,7 @@ Option | Type | Description | Default Value | Supports Repo ID Suffix
`aether.connector.http.preemptivePutAuth` | boolean | Should HTTP client use preemptive-authentication for HTTP PUTs only (works only w/ BASIC). By default is enabled (same as Wagon). | `true` | yes
`aether.connector.http.retryHandler.count` | int | The maximum number of times a request to a remote HTTP server should be retried in case of an error. | `3` | yes
`aether.connector.http.supportWebDav` | boolean | If enabled, transport makes best effort to deploy to WebDAV server. This mode is not recommended, better use real Maven Repository Manager instead. | `false` | yes
+`aether.connector.http.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). This mode is not recommended, better use proper Maven configuration means instead. | `false` | yes
Review Comment:
Originally I wanted to WARN if `proxy != null` (set few lines above), but this builder option is NOT ONLY about proxy but many many other things. Hence the hint "typically used..." as I cannot nor I want to commit to any guarantees here. Simply put: if this option used, user should use it only for "typically set" stuff, and ensure the code is doing what he things is doing.
The reason of existence of this PR is ONLY to allow use of transport-http for use case like https://issues.apache.org/jira/browse/MNG-7721 where user defines only 5 http proxy related properties.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org