Rename Retry options and defaults

docs/reference/urllib3.util.rst

@@ -14,5 +14,4 @@ but can also be used independently.
 
 .. automodule:: urllib3.util
     :members:
-    :undoc-members:
     :show-inheritance:

src/urllib3/util/retry.py

@@ -5,6 +5,7 @@
 from itertools import takewhile
 import email
 import re
+import warnings
 
 from ..exceptions import (
     ConnectTimeoutError,
@@ -27,6 +28,49 @@
 )
 
 
+# TODO: In v2 we can remove this sentinel and metaclass with deprecated options.
+_Default = object()
+
+
+class _RetryMeta(type):
+    @property
+    def DEFAULT_METHOD_WHITELIST(cls):
+        warnings.warn(
+            "Using 'Retry.DEFAULT_METHOD_WHITELIST' is deprecated and "
+            "will be removed in v2.0. Use 'Retry.DEFAULT_METHODS_ALLOWED' instead",
+            DeprecationWarning,
+        )
+        return cls.DEFAULT_ALLOWED_METHODS
+
+    @DEFAULT_METHOD_WHITELIST.setter
+    def DEFAULT_METHOD_WHITELIST(cls, value):
+        warnings.warn(
+            "Using 'Retry.DEFAULT_METHOD_WHITELIST' is deprecated and "
+            "will be removed in v2.0. Use 'Retry.DEFAULT_ALLOWED_METHODS' instead",
+            DeprecationWarning,
+        )
+        cls.DEFAULT_ALLOWED_METHODS = value
+
+    @property
+    def DEFAULT_REDIRECT_HEADERS_BLACKLIST(cls):
+        warnings.warn(
+            "Using 'Retry.DEFAULT_REDIRECT_HEADERS_BLACKLIST' is deprecated and "
+            "will be removed in v2.0. Use 'Retry.DEFAULT_REMOVE_HEADERS_ON_REDIRECT' instead",
+            DeprecationWarning,
+        )
+        return cls.DEFAULT_REMOVE_HEADERS_ON_REDIRECT
+
+    @DEFAULT_REDIRECT_HEADERS_BLACKLIST.setter
+    def DEFAULT_REDIRECT_HEADERS_BLACKLIST(cls, value):
+        warnings.warn(
+            "Using 'Retry.DEFAULT_REDIRECT_HEADERS_BLACKLIST' is deprecated and "
+            "will be removed in v2.0. Use 'Retry.DEFAULT_REMOVE_HEADERS_ON_REDIRECT' instead",
+            DeprecationWarning,
+        )
+        cls.DEFAULT_REMOVE_HEADERS_ON_REDIRECT = value
+
+
+@six.add_metaclass(_RetryMeta)
 class Retry(object):
     """Retry configuration.
 
@@ -107,18 +151,23 @@ class Retry(object):
         If ``total`` is not set, it's a good idea to set this to 0 to account
         for unexpected edge cases and avoid infinite retry loops.
 
-    :param iterable method_whitelist:
+    :param iterable allowed_methods:
         Set of uppercased HTTP method verbs that we should retry on.
 
         By default, we only retry on methods which are considered to be
         idempotent (multiple requests with the same parameters end with the
-        same state). See :attr:`Retry.DEFAULT_METHOD_WHITELIST`.
+        same state). See :attr:`Retry.DEFAULT_ALLOWED_METHODS`.
 
         Set to a ``False`` value to retry on any verb.
 
+        .. warning::
+
+            Previously this parameter was named ``method_whitelist``, that
+            usage is deprecated in v1.26.0 and will be removed in v2.0.
+
     :param iterable status_forcelist:
         A set of integer HTTP status codes that we should force a retry on.
-        A retry is initiated if the request method is in ``method_whitelist``
+        A retry is initiated if the request method is in ``allowed_methods``
         and the response status code is in ``status_forcelist``.
 
         By default, this is disabled with ``None``.
@@ -159,13 +208,16 @@ class Retry(object):
         request.
     """
 
-    DEFAULT_METHOD_WHITELIST = frozenset(
+    #: Default methods to be used for ``allowed_methods``
+    DEFAULT_ALLOWED_METHODS = frozenset(
         ["HEAD", "GET", "PUT", "DELETE", "OPTIONS", "TRACE"]
     )
 
+    #: Default status codes to be used for ``status_forcelist``
     RETRY_AFTER_STATUS_CODES = frozenset([413, 429, 503])
 
-    DEFAULT_REDIRECT_HEADERS_BLACKLIST = frozenset(["Authorization"])
+    #: Default headers to be used for ``remove_headers_on_redirect``
+    DEFAULT_REMOVE_HEADERS_ON_REDIRECT = frozenset(["Authorization"])
 
     #: Maximum backoff time.
     BACKOFF_MAX = 120
@@ -178,16 +230,36 @@ def __init__(
         redirect=None,
         status=None,
         other=None,
-        method_whitelist=DEFAULT_METHOD_WHITELIST,
+        allowed_methods=_Default,
         status_forcelist=None,
         backoff_factor=0,
         raise_on_redirect=True,
         raise_on_status=True,
         history=None,
         respect_retry_after_header=True,
-        remove_headers_on_redirect=DEFAULT_REDIRECT_HEADERS_BLACKLIST,
+        remove_headers_on_redirect=_Default,
+        # TODO: Deprecated, remove in v2.0
+        method_whitelist=_Default,
     ):
 
+        if method_whitelist is not _Default:
+            if allowed_methods is not _Default:
+                raise ValueError(
+                    "Using both 'allowed_methods' and "
+                    "'method_whitelist' together is not allowed. "
+                    "Instead only use 'allowed_methods'"
+                )
+            warnings.warn(
+                "Using 'method_whitelist' with Retry is deprecated and "
+                "will be removed in v2.0. Use 'allowed_methods' instead",
+                DeprecationWarning,
+            )
+            allowed_methods = method_whitelist
+        if allowed_methods is _Default:
+            allowed_methods = self.DEFAULT_ALLOWED_METHODS
+        if remove_headers_on_redirect is _Default:
+            remove_headers_on_redirect = self.DEFAULT_REMOVE_HEADERS_ON_REDIRECT
+
         self.total = total
         self.connect = connect
         self.read = read
@@ -200,7 +272,7 @@ def __init__(
 
         self.redirect = redirect
         self.status_forcelist = status_forcelist or set()
-        self.method_whitelist = method_whitelist
+        self.allowed_methods = allowed_methods
         self.backoff_factor = backoff_factor
         self.raise_on_redirect = raise_on_redirect
         self.raise_on_status = raise_on_status
@@ -218,7 +290,6 @@ def new(self, **kw):
             redirect=self.redirect,
             status=self.status,
             other=self.other,
-            method_whitelist=self.method_whitelist,
             status_forcelist=self.status_forcelist,
             backoff_factor=self.backoff_factor,
             raise_on_redirect=self.raise_on_redirect,
@@ -227,6 +298,23 @@ def new(self, **kw):
             remove_headers_on_redirect=self.remove_headers_on_redirect,
             respect_retry_after_header=self.respect_retry_after_header,
         )
+
+        # TODO: If already given in **kw we use what's given to us
+        # If not given we need to figure out what to pass. We decide
+        # based on whether our class has the 'method_whitelist' property
+        # and if so we pass the deprecated 'method_whitelist' otherwise
+        # we use 'allowed_methods'. Remove in v2.0
+        if "method_whitelist" not in kw and "allowed_methods" not in kw:
+            if "method_whitelist" in self.__dict__:
+                warnings.warn(
+                    "Using 'method_whitelist' with Retry is deprecated and "
+                    "will be removed in v2.0. Use 'allowed_methods' instead",
+                    DeprecationWarning,
+                )
+                params["method_whitelist"] = self.allowed_methods
+            else:
+                params["allowed_methods"] = self.allowed_methods
+
         params.update(kw)
         return type(self)(**params)
 
@@ -340,15 +428,26 @@ def _is_read_error(self, err):
 
     def _is_method_retryable(self, method):
         """Checks if a given HTTP method should be retried upon, depending if
-        it is included on the method whitelist.
+        it is included in the allowed_methods
         """
-        if self.method_whitelist and method.upper() not in self.method_whitelist:
-            return False
+        # TODO: For now favor if the Retry implementation sets its own method_whitelist
+        # property outside of our constructor to avoid breaking custom implementations.
+        if "method_whitelist" in self.__dict__:
+            warnings.warn(
+                "Using 'method_whitelist' with Retry is deprecated and "
+                "will be removed in v2.0. Use 'allowed_methods' instead",
+                DeprecationWarning,
+            )
+            allowed_methods = self.method_whitelist
+        else:
+            allowed_methods = self.allowed_methods
 
+        if allowed_methods and method.upper() not in allowed_methods:
+            return False
         return True
 
     def is_retry(self, method, status_code, has_retry_after=False):
-        """Is this method/status code retryable? (Based on whitelists and control
+        """Is this method/status code retryable? (Based on allowlists and control
         variables such as the number of total retries to allow, whether to
         respect the Retry-After header, whether this header is present, and
         whether the returned status code is on the list of status codes to
@@ -448,7 +547,7 @@ def increment(
 
         else:
             # Incrementing because of a server error like a 500 in
-            # status_forcelist and a the given method is in the whitelist
+            # status_forcelist and the given method is in the allowed_methods
             cause = ResponseError.GENERIC_ERROR
             if response and response.status:
                 if status_count is not None:
@@ -483,6 +582,20 @@ def __repr__(self):
             "read={self.read}, redirect={self.redirect}, status={self.status})"
         ).format(cls=type(self), self=self)
 
+    def __getattr__(self, item):
+        if item == "method_whitelist":
+            # TODO: Remove this deprecated alias in v2.0
+            warnings.warn(
+                "Using 'method_whitelist' with Retry is deprecated and "
+                "will be removed in v2.0. Use 'allowed_methods' instead",
+                DeprecationWarning,
+            )
+            return self.allowed_methods
+        try:
+            return getattr(super(Retry, self), item)
+        except AttributeError:
+            return getattr(Retry, item)
+
 
 # For backwards compatibility (equivalent to pre-v1.9):
 Retry.DEFAULT = Retry(3)

test/test_retry.py

@@ -1,5 +1,6 @@
 import mock
 import pytest
+import warnings
 
 from urllib3.response import HTTPResponse
 from urllib3.packages import six
@@ -15,6 +16,13 @@
 )
 
 
+@pytest.fixture(scope="function", autouse=True)
+def no_retry_deprecations():
+    with warnings.catch_warnings(record=True) as w:
+        yield
+    assert len([str(x.message) for x in w if "Retry" in str(x.message)]) == 0
+
+
 class TestRetry(object):
     def test_string(self):
         """ Retry string representation looks the way we expect """
@@ -196,14 +204,14 @@ def test_status_forcelist(self):
         retry = Retry(total=1, status_forcelist=["418"])
         assert not retry.is_retry("GET", status_code=418)
 
-    def test_method_whitelist_with_status_forcelist(self):
-        # Falsey method_whitelist means to retry on any method.
-        retry = Retry(status_forcelist=[500], method_whitelist=None)
+    def test_allowed_methods_with_status_forcelist(self):
+        # Falsey allowed_methods means to retry on any method.
+        retry = Retry(status_forcelist=[500], allowed_methods=None)
         assert retry.is_retry("GET", status_code=500)
         assert retry.is_retry("POST", status_code=500)
 
-        # Criteria of method_whitelist and status_forcelist are ANDed.
-        retry = Retry(status_forcelist=[500], method_whitelist=["POST"])
+        # Criteria of allowed_methods and status_forcelist are ANDed.
+        retry = Retry(status_forcelist=[500], allowed_methods=["POST"])
         assert not retry.is_retry("GET", status_code=500)
         assert retry.is_retry("POST", status_code=500)
 
@@ -251,7 +259,7 @@ def test_error_message(self):
         assert str(e.value.reason) == "conntimeout"
 
     def test_history(self):
-        retry = Retry(total=10, method_whitelist=frozenset(["GET", "POST"]))
+        retry = Retry(total=10, allowed_methods=frozenset(["GET", "POST"]))
         assert retry.history == tuple()
         connection_error = ConnectTimeoutError("conntimeout")
         retry = retry.increment("GET", "/test1", None, connection_error)