[BTS-1515] Pass PoolTimeout as argument

[apetenchea]

This PR makes the DefaultHTTPClient more flexible, adding the following new parameters:

• request_timeout: socket timeout in seconds for each individual connection
• pool_connections: the number of urllib3 connection pools to cache
• pool_maxsize: the maximum number of connections to save in the pool
• pool_timeout: if set, then the pool will be set to block=True, and requests will block for pool_timeout seconds and raise EmptyPoolError if no connection is available within the time period

The ones prefixed with _pool are all passed to the PoolManager. Note that these values cannot be changed once the HTTP client has been created.

The following constants were already used before, but are now configurable as parameters:

• request_timeout
• retry_attempts
• backoff_factor

Note that request_timeout is mapped to the parameter with the same name of the HTTP client. Unlike the other parameters, this one can be changed.

The default values stay the same. If clients don't specify any parameters, nothing changes.

Example

http_client = DefaultHTTPClient(
    request_timeout=80,
    retry_attempts=5,
    backoff_factor=1.0,
    pool_connections=16,
    pool_maxsize=12,
    pool_timeout=120,
)
client = ArangoClient(hosts="http://127.0.0.1:8529", http_client=http_client)
db = client.db(db.name, username, password, verify=True)
client.request_timeout = 100  # increase timeout for all requests

[codecov-commenter]

Codecov Report

Merging #257 (5e2a717) into main (8b09e07) will decrease coverage by 0.02%.
The diff coverage is 97.72%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##             main     #257      +/-   ##
==========================================
- Coverage   98.58%   98.57%   -0.02%     
==========================================
  Files          26       26              
  Lines        3899     3927      +28     
==========================================
+ Hits         3844     3871      +27     
- Misses         55       56       +1     

Impacted Files	Coverage Δ
arango/client.py	98.30% <75.00%> (-1.70%)	⬇️
arango/aql.py	95.04% <100.00%> (+0.04%)	⬆️
arango/cursor.py	100.00% <100.00%> (ø)
arango/http.py	100.00% <100.00%> (ø)

[jmb-altana]

This is confusingly named. There is an argument to urlopen called pool_timeout which is the timeout to wait for a connection from the pool and only matters if blocking=True. This has nothing to do with the actual connections, just whether requests block to get a cached connection or just make a new one if all cached connections are busy.

You're mapping this to timeout which is a ConnectionPool argument that sets the default connect/read timeout for each connection. But because you set timeout on each request, this pool level default will always be ignored

[jmb-altana]

the result of this is that pool_timeout which maps to timeout on the connection pool, will always be ignored https://urllib3.readthedocs.io/en/stable/reference/urllib3.connectionpool.html#urllib3.HTTPConnectionPool.urlopen

[apetenchea]

Hi @jmb-altana,

Your engagement is highly appreciated!

I've considered your feedback and decided to rename the argument to request_timeout. For the sake of convenience, pool_timeout will be kept around but repurposed to match the equivalent argument of urlopen. Therefore, users have the freedom of tweaking both timeout and pool_timeout parameters. The documentation has also been updated accordingly to reflect these changes.

While request_timeout will be passed as the default value to be used by the ConnectionPool, mapping it to the timeout argument, it's important to note that we can't change this value once the PoolManager is instantiated. However, to keep the ArangoClient backwards compatible and allow for changes to the timeout, we've decided to retain this approach. I believe it offers better flexibility, as it provides the user with the ability to incrementally increase the timeout in case of high network latency. Nevertheless, unless you assign a different request timeout after instantiating the ArangoClient, the ConnectionPool timeout will stay unchanged.

If you have any other requests or opinions, I would be very interested in hearing them. Thank you once again for your input!