Skip to content

Commit

Permalink
method_allowlist -> allowed_methods
Browse files Browse the repository at this point in the history
  • Loading branch information
sethmlarson committed Sep 28, 2020
1 parent 77dd9d8 commit d867f38
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 56 deletions.
60 changes: 30 additions & 30 deletions src/urllib3/util/retry.py
Expand Up @@ -37,19 +37,19 @@ class RetryMeta(type):
def DEFAULT_METHOD_WHITELIST(cls):
warnings.warn(
"Using 'Retry.DEFAULT_METHOD_WHITELIST' is deprecated and "
"will be removed in v2.0. Use 'Retry.DEFAULT_METHOD_ALLOWLIST' instead",
"will be removed in v2.0. Use 'Retry.DEFAULT_METHODS_ALLOWED' instead",
DeprecationWarning,
)
return cls.DEFAULT_METHOD_ALLOWLIST
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_METHOD_ALLOWLIST' instead",
"will be removed in v2.0. Use 'Retry.DEFAULT_ALLOWED_METHODS' instead",
DeprecationWarning,
)
cls.DEFAULT_METHOD_ALLOWLIST = value
cls.DEFAULT_ALLOWED_METHODS = value

@property
def DEFAULT_REDIRECT_HEADERS_BLACKLIST(cls):
Expand Down Expand Up @@ -151,12 +151,12 @@ 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_allowlist:
: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_ALLOWLIST`.
same state). See :attr:`Retry.DEFAULT_ALLOWED_METHODS`.
Set to a ``False`` value to retry on any verb.
Expand All @@ -167,7 +167,7 @@ class Retry(object):
: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_allowlist``
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``.
Expand Down Expand Up @@ -208,8 +208,8 @@ class Retry(object):
request.
"""

#: Default methods to be used for ``method_allowlist``
DEFAULT_METHOD_ALLOWLIST = frozenset(
#: Default methods to be used for ``allowed_methods``
DEFAULT_ALLOWED_METHODS = frozenset(
["HEAD", "GET", "PUT", "DELETE", "OPTIONS", "TRACE"]
)

Expand All @@ -230,7 +230,7 @@ def __init__(
redirect=None,
status=None,
other=None,
method_allowlist=_Default,
allowed_methods=_Default,
status_forcelist=None,
backoff_factor=0,
raise_on_redirect=True,
Expand All @@ -243,20 +243,20 @@ def __init__(
):

if method_whitelist is not _Default:
if method_allowlist is not _Default:
if allowed_methods is not _Default:
raise ValueError(
"Using both 'method_allowlist' and "
"Using both 'allowed_methods' and "
"'method_whitelist' together is not allowed. "
"Instead only use 'method_allowlist'"
"Instead only use 'allowed_methods'"
)
warnings.warn(
"Using 'method_whitelist' with Retry is deprecated and "
"will be removed in v2.0. Use 'method_allowlist' instead",
"will be removed in v2.0. Use 'allowed_methods' instead",
DeprecationWarning,
)
method_allowlist = method_whitelist
if method_allowlist is _Default:
method_allowlist = self.DEFAULT_METHOD_ALLOWLIST
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

Expand All @@ -272,7 +272,7 @@ def __init__(

self.redirect = redirect
self.status_forcelist = status_forcelist or set()
self.method_allowlist = method_allowlist
self.allowed_methods = allowed_methods
self.backoff_factor = backoff_factor
self.raise_on_redirect = raise_on_redirect
self.raise_on_status = raise_on_status
Expand Down Expand Up @@ -303,17 +303,17 @@ def new(self, **kw):
# 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 'method_allowlist'. Remove in v2.0
if "method_whitelist" not in kw and "method_allowlist" not in kw:
# 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 'method_allowlist' instead",
"will be removed in v2.0. Use 'allowed_methods' instead",
DeprecationWarning,
)
kw["method_whitelist"] = self.method_allowlist
params["method_whitelist"] = self.allowed_methods
else:
kw["method_allowlist"] = self.method_allowlist
params["allowed_methods"] = self.allowed_methods

params.update(kw)
return type(self)(**params)
Expand Down Expand Up @@ -435,14 +435,14 @@ def _is_method_retryable(self, method):
if "method_whitelist" in self.__dict__:
warnings.warn(
"Using 'method_whitelist' with Retry is deprecated and "
"will be removed in v2.0. Use 'method_allowlist' instead",
"will be removed in v2.0. Use 'allowed_methods' instead",
DeprecationWarning,
)
method_allowlist = self.method_whitelist
allowed_methods = self.method_whitelist
else:
method_allowlist = self.method_allowlist
allowed_methods = self.allowed_methods

if method_allowlist and method.upper() not in method_allowlist:
if allowed_methods and method.upper() not in allowed_methods:
return False
return True

Expand Down Expand Up @@ -547,7 +547,7 @@ def increment(

else:
# Incrementing because of a server error like a 500 in
# status_forcelist and the given method is in the method_allowlist
# 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:
Expand Down Expand Up @@ -587,10 +587,10 @@ def __getattr__(self, item):
# 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 'method_allowlist' instead",
"will be removed in v2.0. Use 'allowed_methods' instead",
DeprecationWarning,
)
return self.method_allowlist
return self.allowed_methods
try:
return getattr(super(Retry, self), item)
except AttributeError:
Expand Down
12 changes: 6 additions & 6 deletions test/test_retry.py
Expand Up @@ -204,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_allowlist_with_status_forcelist(self):
# Falsey method_allowlist means to retry on any method.
retry = Retry(status_forcelist=[500], method_allowlist=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_allowlist and status_forcelist are ANDed.
retry = Retry(status_forcelist=[500], method_allowlist=["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)

Expand Down Expand Up @@ -259,7 +259,7 @@ def test_error_message(self):
assert str(e.value.reason) == "conntimeout"

def test_history(self):
retry = Retry(total=10, method_allowlist=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)
Expand Down
40 changes: 20 additions & 20 deletions test/test_retry_deprecated.py
Expand Up @@ -374,7 +374,7 @@ def test_respect_retry_after_header_sleep(

class TestRetryDeprecations(object):
def test_cls_get_default_method_whitelist(self, expect_retry_deprecation):
assert Retry.DEFAULT_METHOD_ALLOWLIST == Retry.DEFAULT_METHOD_WHITELIST
assert Retry.DEFAULT_ALLOWED_METHODS == Retry.DEFAULT_METHOD_WHITELIST

def test_cls_get_default_redirect_headers_blacklist(self, expect_retry_deprecation):
assert (
Expand All @@ -387,26 +387,26 @@ def test_cls_set_default_method_whitelist(self, expect_retry_deprecation):
try:
Retry.DEFAULT_METHOD_WHITELIST = {"GET"}
retry = Retry()
assert retry.DEFAULT_METHOD_ALLOWLIST == {"GET"}
assert retry.DEFAULT_ALLOWED_METHODS == {"GET"}
assert retry.DEFAULT_METHOD_WHITELIST == {"GET"}
assert retry.method_allowlist == {"GET"}
assert retry.allowed_methods == {"GET"}
assert retry.method_whitelist == {"GET"}

# Test that the default can be overridden both ways
retry = Retry(method_allowlist={"GET", "POST"})
assert retry.DEFAULT_METHOD_ALLOWLIST == {"GET"}
retry = Retry(allowed_methods={"GET", "POST"})
assert retry.DEFAULT_ALLOWED_METHODS == {"GET"}
assert retry.DEFAULT_METHOD_WHITELIST == {"GET"}
assert retry.method_allowlist == {"GET", "POST"}
assert retry.allowed_methods == {"GET", "POST"}
assert retry.method_whitelist == {"GET", "POST"}

retry = Retry(method_whitelist={"POST"})
assert retry.DEFAULT_METHOD_ALLOWLIST == {"GET"}
assert retry.DEFAULT_ALLOWED_METHODS == {"GET"}
assert retry.DEFAULT_METHOD_WHITELIST == {"GET"}
assert retry.method_allowlist == {"POST"}
assert retry.allowed_methods == {"POST"}
assert retry.method_whitelist == {"POST"}
finally:
Retry.DEFAULT_METHOD_WHITELIST = old_setting
assert Retry.DEFAULT_METHOD_ALLOWLIST == old_setting
assert Retry.DEFAULT_ALLOWED_METHODS == old_setting

def test_cls_set_default_redirect_headers_blacklist(self, expect_retry_deprecation):
old_setting = Retry.DEFAULT_REDIRECT_HEADERS_BLACKLIST
Expand All @@ -430,40 +430,40 @@ def test_cls_set_default_redirect_headers_blacklist(self, expect_retry_deprecati
@pytest.mark.parametrize(
"options", [(None, None), ({"GET"}, None), (None, {"GET"}), ({"GET"}, {"GET"})]
)
def test_retry_method_allowlist_and_whitelist_error(self, options):
def test_retry_allowed_methods_and_method_whitelist_error(self, options):
with pytest.raises(ValueError) as e:
Retry(method_allowlist=options[0], method_whitelist=options[1])
Retry(allowed_methods=options[0], method_whitelist=options[1])
assert str(e.value) == (
"Using both 'method_allowlist' and 'method_whitelist' together "
"is not allowed. Instead only use 'method_allowlist'"
"Using both 'allowed_methods' and 'method_whitelist' together "
"is not allowed. Instead only use 'allowed_methods'"
)

def test_retry_subclass_that_sets_method_whitelist(self, expect_retry_deprecation):
class SubclassRetry(Retry):
def __init__(self, **kwargs):
if "method_allowlist" in kwargs:
if "allowed_methods" in kwargs:
raise AssertionError(
"This subclass likely doesn't use 'method_allowlist'"
"This subclass likely doesn't use 'allowed_methods'"
)

super(SubclassRetry, self).__init__(**kwargs)

# Since we're setting 'method_whiteist' we get fallbacks
# within Retry.new() and Retry._is_method_retryable()
# to use 'method_whitelist' instead of 'method_allowlist'
# to use 'method_whitelist' instead of 'allowed_methods'
self.method_whitelist = self.method_whitelist | {"POST"}

retry = SubclassRetry()
assert retry.method_whitelist == Retry.DEFAULT_METHOD_ALLOWLIST | {"POST"}
assert retry.method_whitelist == Retry.DEFAULT_ALLOWED_METHODS | {"POST"}
assert retry.new(read=0).method_whitelist == retry.method_whitelist
assert retry._is_method_retryable("POST")
assert not retry._is_method_retryable("CONNECT")

assert retry.new(method_whitelist={"GET"}).method_whitelist == {"GET", "POST"}

# urllib3 doesn't do this during normal operation
# so we don't want users passing in 'method_allowlist'
# so we don't want users passing in 'allowed_methods'
# when their subclass doesn't support the option yet.
with pytest.raises(AssertionError) as e:
retry.new(method_allowlist={"GET"})
assert str(e.value) == "This subclass likely doesn't use 'method_allowlist'"
retry.new(allowed_methods={"GET"})
assert str(e.value) == "This subclass likely doesn't use 'allowed_methods'"

0 comments on commit d867f38

Please sign in to comment.