openstack/keystoneauth
Code Issues Proposed changes

Add optional support for retrying certain HTTP codes

Browse Source 
Ironic commonly returns HTTP 409 when a node is locked by another routine
and HTTP 503 when the conductor has no free threads to process the request.
Currently it is managed by custom code in ironicclient and openstacksdk,
this change will allow to move it to Session itself.

Change-Id: I04e356e7856b020cd20aa598e291ef31e02730d2
This commit is contained in:
Dmitry Tantsur 2018-05-29 14:54:56 +02:00
parent db5aa8b3ae
commit 3c2cf44e1c
4 changed files with 150 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
keystoneauth1/adapter.py
View File

@ -80,6 +80,14 @@ class Adapter(object):
a per-request feature, a user may know they
want to default to sending a specific value.
(optional)
:param int status_code_retries: the maximum number of retries that
should be attempted for retriable
HTTP status codes (optional, defaults
to 0 - never retry).
:param list retriable_status_codes: list of HTTP status codes that
should be retried (optional,
defaults to HTTP 503, has no effect
when status_code_retries is 0).
"""
client_name = None
@ -93,7 +101,8 @@ class Adapter(object):
client_version=None, allow_version_hack=None,
global_request_id=None,
min_version=None, max_version=None,
default_microversion=None):
default_microversion=None, status_code_retries=None,
retriable_status_codes=None):
if version and (min_version or max_version):
raise TypeError(
"version is mutually exclusive with min_version and"
@ -120,6 +129,8 @@ class Adapter(object):
self.min_version = min_version
self.max_version = max_version
self.default_microversion = default_microversion
self.status_code_retries = status_code_retries
self.retriable_status_codes = retriable_status_codes
self.global_request_id = global_request_id
@ -159,6 +170,11 @@ class Adapter(object):
kwargs.setdefault('user_agent', self.user_agent)
if self.connect_retries is not None:
kwargs.setdefault('connect_retries', self.connect_retries)
if self.status_code_retries is not None:
kwargs.setdefault('status_code_retries', self.status_code_retries)
if self.retriable_status_codes:
kwargs.setdefault('retriable_status_codes',
self.retriable_status_codes)
if self.logger:
kwargs.setdefault('logger', self.logger)
if self.allow:

40
keystoneauth1/session.py
View File

@ -560,6 +560,7 @@ class Session(object):
endpoint_override=None, connect_retries=0, logger=None,
allow=None, client_name=None, client_version=None,
microversion=None, microversion_service_type=None,
status_code_retries=0, retriable_status_codes=None,
**kwargs):
"""Send an HTTP request with the specified characteristics.
@ -638,6 +639,14 @@ class Session(object):
provided or does not have a service_type, microversion
is given and microversion_service_type is not provided,
an exception will be raised.
:param int status_code_retries: the maximum number of retries that
should be attempted for retriable
HTTP status codes (optional, defaults
to 0 - never retry).
:param list retriable_status_codes: list of HTTP status codes that
should be retried (optional,
defaults to HTTP 503, has no effect
when status_code_retries is 0).
:param kwargs: any other parameter that can be passed to
:meth:`requests.Session.request` (such as `headers`).
Except:
@ -659,6 +668,8 @@ class Session(object):
else:
split_loggers = None
logger = logger or utils.get_logger(__name__)
# HTTP 503 - Service Unavailable
retriable_status_codes = retriable_status_codes or [503]
headers = kwargs.setdefault('headers', dict())
if microversion:
@ -785,7 +796,8 @@ class Session(object):
send = functools.partial(self._send_request,
url, method, redirect, log, logger,
split_loggers, connect_retries)
split_loggers, connect_retries,
status_code_retries, retriable_status_codes)
try:
connection_params = self.get_auth_connection_params(auth=auth)
@ -872,7 +884,9 @@ class Session(object):
return resp
def _send_request(self, url, method, redirect, log, logger, split_loggers,
connect_retries, connect_retry_delay=0.5, **kwargs):
connect_retries, status_code_retries,
retriable_status_codes, connect_retry_delay=0.5,
status_code_retry_delay=0.5, **kwargs):
# NOTE(jamielennox): We handle redirection manually because the
# requests lib follows some browser patterns where it will redirect
# POSTs as GETs for certain statuses which is not want we want for an
@ -918,6 +932,8 @@ class Session(object):
return self._send_request(
url, method, redirect, log, logger, split_loggers,
status_code_retries=status_code_retries,
retriable_status_codes=retriable_status_codes,
connect_retries=connect_retries - 1,
connect_retry_delay=connect_retry_delay * 2,
**kwargs)
@ -949,12 +965,32 @@ class Session(object):
new_resp = self._send_request(
location, method, redirect, log, logger, split_loggers,
connect_retries=connect_retries,
status_code_retries=status_code_retries,
retriable_status_codes=retriable_status_codes,
**kwargs)
if not isinstance(new_resp.history, list):
new_resp.history = list(new_resp.history)
new_resp.history.insert(0, resp)
resp = new_resp
elif (resp.status_code in retriable_status_codes and
status_code_retries > 0):
logger.info('Retriable status code %(code)s. Retrying in '
'%(delay).1fs.',
{'code': resp.status_code,
'delay': status_code_retry_delay})
time.sleep(status_code_retry_delay)
# NOTE(jamielennox): We don't pass through connect_retry_delay.
# This request actually worked so we can reset the delay count.
return self._send_request(
url, method, redirect, log, logger, split_loggers,
connect_retries=connect_retries,
status_code_retries=status_code_retries - 1,
retriable_status_codes=retriable_status_codes,
status_code_retry_delay=status_code_retry_delay * 2,
**kwargs)
return resp

89
keystoneauth1/tests/unit/test_session.py
View File

@ -440,6 +440,62 @@ class SessionTests(utils.TestCase):
self.assertThat(self.requests_mock.request_history,
matchers.HasLength(retries + 1))
def test_http_503_retries(self):
self.stub_url('GET', status_code=503)
session = client_session.Session()
retries = 3
with mock.patch('time.sleep') as m:
self.assertRaises(exceptions.ServiceUnavailable,
session.get,
self.TEST_URL, status_code_retries=retries)
self.assertEqual(retries, m.call_count)
# 3 retries finishing with 2.0 means 0.5, 1.0 and 2.0
m.assert_called_with(2.0)
# we count retries so there will be one initial request + 3 retries
self.assertThat(self.requests_mock.request_history,
matchers.HasLength(retries + 1))
def test_http_status_retries(self):
self.stub_url('GET', status_code=409)
session = client_session.Session()
retries = 3
with mock.patch('time.sleep') as m:
self.assertRaises(exceptions.Conflict,
session.get,
self.TEST_URL, status_code_retries=retries,
retriable_status_codes=[503, 409])
self.assertEqual(retries, m.call_count)
# 3 retries finishing with 2.0 means 0.5, 1.0 and 2.0
m.assert_called_with(2.0)
# we count retries so there will be one initial request + 3 retries
self.assertThat(self.requests_mock.request_history,
matchers.HasLength(retries + 1))
def test_http_status_retries_another_code(self):
self.stub_url('GET', status_code=404)
session = client_session.Session()
retries = 3
with mock.patch('time.sleep') as m:
self.assertRaises(exceptions.NotFound,
session.get,
self.TEST_URL, status_code_retries=retries,
retriable_status_codes=[503, 409])
self.assertFalse(m.called)
self.assertThat(self.requests_mock.request_history,
matchers.HasLength(1))
def test_uses_tcp_keepalive_by_default(self):
session = client_session.Session()
requests_session = session.session
@ -1217,6 +1273,39 @@ class AdapterTest(utils.TestCase):
self.assertThat(self.requests_mock.request_history,
matchers.HasLength(retries + 1))
def test_adapter_http_503_retries(self):
retries = 2
sess = client_session.Session()
adpt = adapter.Adapter(sess, status_code_retries=retries)
self.stub_url('GET', status_code=503)
with mock.patch('time.sleep') as m:
self.assertRaises(exceptions.ServiceUnavailable,
adpt.get, self.TEST_URL)
self.assertEqual(retries, m.call_count)
# we count retries so there will be one initial request + 2 retries
self.assertThat(self.requests_mock.request_history,
matchers.HasLength(retries + 1))
def test_adapter_http_status_retries(self):
retries = 2
sess = client_session.Session()
adpt = adapter.Adapter(sess, status_code_retries=retries,
retriable_status_codes=[503, 409])
self.stub_url('GET', status_code=409)
with mock.patch('time.sleep') as m:
self.assertRaises(exceptions.Conflict,
adpt.get, self.TEST_URL)
self.assertEqual(retries, m.call_count)
# we count retries so there will be one initial request + 2 retries
self.assertThat(self.requests_mock.request_history,
matchers.HasLength(retries + 1))
def test_user_and_project_id(self):
auth = AuthPlugin()
sess = client_session.Session()

6
releasenotes/notes/status-code-retries-75052a43efa4edb2.yaml Normal file
View File

@ -0,0 +1,6 @@
---
features:
- |
Addes support for retrying certain HTTP status codes when doing requests
via the new ``status_code_retries`` and ``retriable_status_codes``
parameters for ``Session`` and ``Adapter``.