carterkozak commented on a change in pull request #296:
URL: 
https://github.com/apache/httpcomponents-client/pull/296#discussion_r601615151



##########
File path: 
httpclient5/src/main/java/org/apache/hc/client5/http/config/RequestConfig.java
##########
@@ -43,7 +43,6 @@
 public class RequestConfig implements Cloneable {
 
     private static final Timeout DEFAULT_CONNECTION_REQUEST_TIMEOUT = 
Timeout.ofMinutes(3);
-    private static final Timeout DEFAULT_CONNECT_TIMEOUT = 
Timeout.ofMinutes(3);
     private static final TimeValue DEFAULT_CONN_KEEP_ALIVE = 
TimeValue.ofMinutes(3);

Review comment:
       Based on the request and response I think there are three results:
   1. Do not keep the connection alive (`Connection: close` response header)
   2. keep the connection alive for a specific amount of time `Keep-Alive: 
timeout=30`
   3. keep the connection alive for an indeterminate amount of time so the 
default value can be used (http/1.1 without specifying close)
   
   If memory serves, we use `null` for [1] (connection is not kept alive) and 
return a value based on either a keep-alive timeout value [2] or the default 
[3] when configured. If we can update the abstraction to differentiate states 
[1] from [3] I think the default configuration makes sense in the connection 
management configuration.




-- 
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.

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



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

Reply via email to