@nchaigne commented on this pull request.
In src/modules/http_client/doc/http_client_admin.xml:
> @@ -187,15 +187,35 @@ modparam("http_client", "maxdatasize", 2000) </para> <para> <emphasis> - Default value is zero, i.e., - the timeout function is disabled. + Default value is 4.
I am for adding
timeout_mode
, seems cleaner to understand, imo. The only thing on your proposal that breaks the existing functionality is the ability to disable the timeout. I agree that there should be a timeout by default and I cannot think of a use case for disabling it (maybe when it is sure the http service is available(?!?) and not setting it saves some cpu/timer cycles), so I would propose the following:
- if timeout_mode is 0, then no timeout is set (disabled)
- if timeout_mode is 1, then timeout value is in seconds and if connection_timeout is not set, then it is set to 4 (default), setting CURLOPT_TIMEOUT
- if timeout_mode is 2, then timeout value is in milliseconds and if connection_timeout is not set, then it is set to 4000, setting CURLOPT_TIMEOUT_MS
This seems a good idea. I will then implement this new timeout_mode behaviour.
In addition, if timeout_mode is 0, I propose to fixup all connections to force their "timeout" to 0 (to reflect the fact that no timeout will be handled - this would be useful when using RPC command "httpclient.listcon").
To recap:
default global timeout = 0 (unconfigured).
Parse connections as usual. If they have a timeout configured, use it.
In mod_init:
if global timeout == 0 (unconfigured), and timeout_mode is 1 or 2:
if timeout_mode == 1 -> global timeout = 4 (seconds)
if timeout_mode == 2 -> global timeout = 4000 (milliseconds)
for each connection "conn" (fixup):
if timeout_mode is not 1 or 2 -> conn.timeout = 0 (to reflect the fact that no timeout will be handled)
else if conn.timeout is not configured -> conn.timeout = global timeout (in seconds or milliseconds, depending on timeout_mode).
When doing Curl requests (curL_request_url):
if timeout_mode == 1: set CURLOPT_TIMEOUT
if timeout_mode == 2: set CURLOPT_TIMEOUT_MS
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.