You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by "arturobernalg (via GitHub)" <gi...@apache.org> on 2025/07/17 14:49:41 UTC

[PR] HTTPCLIENT-2381 Enable automatic mapping [httpcomponents-client]

arturobernalg opened a new pull request, #680:
URL: https://github.com/apache/httpcomponents-client/pull/680

   Enable env-proxy support ([HTTPCLIENT-2381](Enable env-proxy support (HTTPCLIENT-2381) – add EnvironmentProxyConfigurer, wire it into HttpClientBuilder, revert extra fallbacks, and introduce JUnit test (System-Lambda) with dep-management cleanup.)) – add `EnvironmentProxyConfigurer`, wire it into `HttpClientBuilder`, revert extra fallbacks, and introduce JUnit test (System-Lambda) with dep-management cleanup.


-- 
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: dev-unsubscribe@hc.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


Re: [PR] HTTPCLIENT-2381 Enable automatic mapping [httpcomponents-client]

Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c commented on code in PR #680:
URL: https://github.com/apache/httpcomponents-client/pull/680#discussion_r2216502806


##########
httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpClientBuilder.java:
##########
@@ -838,6 +839,11 @@ protected Function<HttpContext, HttpClientContext> contextAdaptor() {
     }
 
     public CloseableHttpClient build() {
+

Review Comment:
   @arturobernalg I am not sure I see what has changed. `EnvironmentProxyConfigurer#apply` call is still there instead of “do nothing unless asked”.



-- 
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: dev-unsubscribe@hc.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


Re: [PR] HTTPCLIENT-2381 Enable automatic mapping [httpcomponents-client]

Posted by "arturobernalg (via GitHub)" <gi...@apache.org>.
arturobernalg closed pull request #680: HTTPCLIENT-2381 Enable automatic mapping 
URL: https://github.com/apache/httpcomponents-client/pull/680


-- 
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: dev-unsubscribe@hc.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


Re: [PR] HTTPCLIENT-2381 Enable automatic mapping [httpcomponents-client]

Posted by "arturobernalg (via GitHub)" <gi...@apache.org>.
arturobernalg commented on code in PR #680:
URL: https://github.com/apache/httpcomponents-client/pull/680#discussion_r2216032226


##########
httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpClientBuilder.java:
##########
@@ -838,6 +839,11 @@ protected Function<HttpContext, HttpClientContext> contextAdaptor() {
     }
 
     public CloseableHttpClient build() {
+

Review Comment:
   @ok2c  Oleg, one last thought before I push: would you prefer we gate the transfer behind a dedicated flag, e.g. `hc5_apply_proxy_env=true` so users who really want the mapping can simply set it and skip the
   explicit `EnvironmentProxyConfigurer.apply()` call?
   Default remains “do nothing unless asked”



-- 
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: dev-unsubscribe@hc.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


Re: [PR] HTTPCLIENT-2381 Enable automatic mapping [httpcomponents-client]

Posted by "arturobernalg (via GitHub)" <gi...@apache.org>.
arturobernalg commented on code in PR #680:
URL: https://github.com/apache/httpcomponents-client/pull/680#discussion_r2216440598


##########
httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpClientBuilder.java:
##########
@@ -838,6 +839,11 @@ protected Function<HttpContext, HttpClientContext> contextAdaptor() {
     }
 
     public CloseableHttpClient build() {
+

Review Comment:
   @ok2c  please do anther pass



-- 
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: dev-unsubscribe@hc.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


Re: [PR] HTTPCLIENT-2381 Enable automatic mapping [httpcomponents-client]

Posted by "arturobernalg (via GitHub)" <gi...@apache.org>.
arturobernalg commented on code in PR #680:
URL: https://github.com/apache/httpcomponents-client/pull/680#discussion_r2218556411


##########
httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpClientBuilder.java:
##########
@@ -237,6 +237,9 @@ private ExecInterceptorEntry(
 
     private List<Closeable> closeables;
 
+    private boolean applyEnvProxy;

Review Comment:
   > @arturobernalg This should no longer be necessary, should it?
   
   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: dev-unsubscribe@hc.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


Re: [PR] HTTPCLIENT-2381 Enable automatic mapping [httpcomponents-client]

Posted by "arturobernalg (via GitHub)" <gi...@apache.org>.
arturobernalg commented on code in PR #680:
URL: https://github.com/apache/httpcomponents-client/pull/680#discussion_r2216032226


##########
httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpClientBuilder.java:
##########
@@ -838,6 +839,11 @@ protected Function<HttpContext, HttpClientContext> contextAdaptor() {
     }
 
     public CloseableHttpClient build() {
+

Review Comment:
   @ok2c  Oleg, one last thought before I push: would you prefer we gate the transfer behind a dedicated env-flag, e.g. `hc5_apply_proxy_env=true` so users who really want the mapping can simply set it and skip the
   explicit `EnvironmentProxyConfigurer.apply()` call?
   Default remains “do nothing unless asked”



-- 
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: dev-unsubscribe@hc.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


Re: [PR] HTTPCLIENT-2381 Enable automatic mapping [httpcomponents-client]

Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c commented on PR #680:
URL: https://github.com/apache/httpcomponents-client/pull/680#issuecomment-3094643798

   @arturobernalg Actually I think it misplaced. This logic should go into `SystemDefaultRoutePlanner` and our code must not change the system properties _at all_ and populate `HttpRoute`s instead based on environment


-- 
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: dev-unsubscribe@hc.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


Re: [PR] HTTPCLIENT-2381 Enable automatic mapping [httpcomponents-client]

Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c commented on code in PR #680:
URL: https://github.com/apache/httpcomponents-client/pull/680#discussion_r2216229648


##########
httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpClientBuilder.java:
##########
@@ -838,6 +839,11 @@ protected Function<HttpContext, HttpClientContext> contextAdaptor() {
     }
 
     public CloseableHttpClient build() {
+

Review Comment:
   @arturobernalg I do not have a use case, so I can make this call. _Personally_ I would prefer just an explicit `EnvironmentProxyConfigurer#apply` call, but I leave it up to you.



-- 
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: dev-unsubscribe@hc.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


Re: [PR] HTTPCLIENT-2381 Enable automatic mapping [httpcomponents-client]

Posted by "arturobernalg (via GitHub)" <gi...@apache.org>.
arturobernalg commented on code in PR #680:
URL: https://github.com/apache/httpcomponents-client/pull/680#discussion_r2217300779


##########
httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpClientBuilder.java:
##########
@@ -838,6 +839,11 @@ protected Function<HttpContext, HttpClientContext> contextAdaptor() {
     }
 
     public CloseableHttpClient build() {
+

Review Comment:
   please @ok2c  do another pass



-- 
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: dev-unsubscribe@hc.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


Re: [PR] HTTPCLIENT-2381 Enable automatic mapping [httpcomponents-client]

Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c commented on code in PR #680:
URL: https://github.com/apache/httpcomponents-client/pull/680#discussion_r2218341837


##########
httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpClientBuilder.java:
##########
@@ -237,6 +237,9 @@ private ExecInterceptorEntry(
 
     private List<Closeable> closeables;
 
+    private boolean applyEnvProxy;

Review Comment:
   @arturobernalg This should no longer be necessary, should it?



-- 
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: dev-unsubscribe@hc.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


Re: [PR] HTTPCLIENT-2381 Enable automatic mapping [httpcomponents-client]

Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c commented on code in PR #680:
URL: https://github.com/apache/httpcomponents-client/pull/680#discussion_r2215809120


##########
httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpClientBuilder.java:
##########
@@ -838,6 +839,11 @@ protected Function<HttpContext, HttpClientContext> contextAdaptor() {
     }
 
     public CloseableHttpClient build() {
+

Review Comment:
   @arturobernalg I am a bit unsure HttpClient should be meddling with system variables behind the scene. This can really have unexpected side-effects. Could we document `EnvironmentProxyConfigurer` in javadocs and advise the users to explicitly call `EnvironmentProxyConfigurer#apply` in order to carry over proxy config from the runtime environment? 



-- 
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: dev-unsubscribe@hc.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org