PoolManager retries argument is not passed down to requests

[vbarbaresi]

Subject

The documentation states that retries argument can be passed to a PoolManager to be used as default in pool requests: https://urllib3.readthedocs.io/en/latest/reference/urllib3.util.html#utilities

It seems that retries is actually ignored in the pool requests

Environment

OS macOS-10.15.3-x86_64-i386-64bit
Python 3.8.6
urllib3 1.26.7

Also reproduced in development mode in urllib3 main branch

Steps to Reproduce

from urllib3 import Retry, PoolManager
from urllib3.exceptions import MaxRetryError

retries = Retry(connect=0, read=0, redirect=0)
http = PoolManager(retries=retries)

# Max redirect retries not honored
response = http.request("GET", "http://nghttp2.org/httpbin/redirect/2")
assert response.status == 200
print("Should have raised MaxRetryError")

try:
    http.request("GET", "http://nghttp2.org/httpbin/redirect/3", retries=retries)
except MaxRetryError:
    print("Max retry honored")

Expected Behavior

I would expect the first request to raise MaxRetryError

Actual Behavior

The first request actually use a default retry strategy instead of the pool's strategy

diff --git a/src/urllib3/poolmanager.py b/src/urllib3/poolmanager.py
index a36f96ae..70ebc65e 100644
--- a/src/urllib3/poolmanager.py
+++ b/src/urllib3/poolmanager.py
@@ -453,7 +453,7 @@ class PoolManager(RequestMethods):
         if response.status == 303:
             method = "GET"

-        retries = kw.get("retries")
+        retries = kw.get("retries") or self.connection_pool_kw.get("retries")
         if not isinstance(retries, Retry):
             retries = Retry.from_int(retries, redirect=redirect)

This fixes the issue. Does that seem like the right way to fix this? I can write a PR and a unit test. Or maybe connection_pool_kw arguments should be merged in a more generic place like _merge_pool_kwargs().

[misha778]

Thank you! it's a pity that doesn't help for requests, it doesn't seem to be using the poolmanager urlopen method.

[misha778]

with a timeout error, retry is working fine

retry = Retry(total=10)
http = urllib3.PoolManager(retries=retry, timeout=2)

r = http.request("GET", "https://httpbin.org/delay/5")

[vbarbaresi]

Indeed, for timeouts it goes through a different codepath in connectionpool.urlopen()

It default on self.retries when retries is not passed as keyword argument: https://github.com/urllib3/urllib3/blob/fac7db19a1e8e15e5b302e2203ce35cd0485bc88/src/urllib3/connectionpool.py#L642-L643

So this only affects other type of errors like redirects (in poolmanager.py)