summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJulien Danjou <julien@danjou.info>2016-06-07 15:49:32 +0200
committerJulien Danjou <julien@danjou.info>2016-06-07 16:58:26 +0200
commita62b7ee06c1a300c73d9a01b2c9bc05028f1fabd (patch)
treeecaa70376eeec43488af354d5883d3cc3ef4d34a
parentc91c8d575e86b480270f8dfb7df49355dd38ee54 (diff)
downloadpython-swiftclient-a62b7ee06c1a300c73d9a01b2c9bc05028f1fabd.tar.gz
client: renew token on 401 even if retries is 0
Gnocchi uses a client with retries=0 to maximize throughtput and not retry N times on e.g. 404 when checking existence of an object. However, this as the side effect of never renewing the token since there' no retry on 401 either. This patches change the behavior so that 401 errors are always retried, whatever the retries value is. Closes-Bug: #1589926 Change-Id: Ie06adf4cf17ea4592b5bbd7bbde9828e5e134e3e
-rw-r--r--swiftclient/client.py8
-rw-r--r--tests/unit/test_swiftclient.py78
2 files changed, 82 insertions, 4 deletions
diff --git a/swiftclient/client.py b/swiftclient/client.py
index 744a876..f556afd 100644
--- a/swiftclient/client.py
+++ b/swiftclient/client.py
@@ -1544,7 +1544,7 @@ class Connection(object):
backoff = self.starting_backoff
caller_response_dict = kwargs.pop('response_dict', None)
self.attempts = kwargs.pop('attempts', 0)
- while self.attempts <= self.retries:
+ while self.attempts <= self.retries or retried_auth:
self.attempts += 1
try:
if not self.url or not self.token:
@@ -1573,9 +1573,6 @@ class Connection(object):
self.http_conn = None
except ClientException as err:
self._add_response_dict(caller_response_dict, kwargs)
- if self.attempts > self.retries or err.http_status is None:
- logger.exception(err)
- raise
if err.http_status == 401:
self.url = self.token = self.service_token = None
if retried_auth or not all((self.authurl,
@@ -1584,6 +1581,9 @@ class Connection(object):
logger.exception(err)
raise
retried_auth = True
+ elif self.attempts > self.retries or err.http_status is None:
+ logger.exception(err)
+ raise
elif err.http_status == 408:
self.http_conn = None
elif 500 <= err.http_status <= 599:
diff --git a/tests/unit/test_swiftclient.py b/tests/unit/test_swiftclient.py
index cbb95db..4a39458 100644
--- a/tests/unit/test_swiftclient.py
+++ b/tests/unit/test_swiftclient.py
@@ -2556,6 +2556,84 @@ class TestServiceToken(MockHttpTest):
self.assertEqual('service_project_name',
auth_kwargs['os_options']['tenant_name'])
+ def test_service_token_reauth_retries_0(self):
+ get_auth_call_list = []
+
+ def get_auth(url, user, key, **kwargs):
+ # The real get_auth function will always return the os_option
+ # dict's object_storage_url which will be overridden by the
+ # preauthurl parameter to Connection if it is provided.
+ args = {'url': url, 'user': user, 'key': key, 'kwargs': kwargs}
+ get_auth_call_list.append(args)
+ return_dict = {'asdf': 'new', 'service_username': 'newserv'}
+ storage_url = kwargs['os_options'].get('object_storage_url')
+ return storage_url, return_dict[user]
+
+ def swap_sleep(*args):
+ self.swap_sleep_called = True
+ c.get_auth = get_auth
+
+ with mock.patch('swiftclient.client.http_connection',
+ self.fake_http_connection(401, 200)):
+ with mock.patch('swiftclient.client.sleep', swap_sleep):
+ self.swap_sleep_called = False
+
+ conn = c.Connection('http://www.test.com', 'asdf', 'asdf',
+ preauthurl='http://www.old.com',
+ preauthtoken='old',
+ os_options=self.os_options,
+ retries=0)
+
+ self.assertEqual(conn.attempts, 0)
+ self.assertEqual(conn.url, 'http://www.old.com')
+ self.assertEqual(conn.token, 'old')
+
+ conn.head_account()
+
+ self.assertTrue(self.swap_sleep_called)
+ self.assertEqual(conn.attempts, 2)
+ # The original 'preauth' storage URL *must* be preserved
+ self.assertEqual(conn.url, 'http://www.old.com')
+ self.assertEqual(conn.token, 'new')
+ self.assertEqual(conn.service_token, 'newserv')
+
+ # Check get_auth was called with expected args
+ auth_args = get_auth_call_list[0]
+ auth_kwargs = get_auth_call_list[0]['kwargs']
+ self.assertEqual('asdf', auth_args['user'])
+ self.assertEqual('asdf', auth_args['key'])
+ self.assertEqual('service_key',
+ auth_kwargs['os_options']['service_key'])
+ self.assertEqual('service_username',
+ auth_kwargs['os_options']['service_username'])
+ self.assertEqual('service_project_name',
+ auth_kwargs['os_options']['service_project_name'])
+
+ auth_args = get_auth_call_list[1]
+ auth_kwargs = get_auth_call_list[1]['kwargs']
+ self.assertEqual('service_username', auth_args['user'])
+ self.assertEqual('service_key', auth_args['key'])
+ self.assertEqual('service_project_name',
+ auth_kwargs['os_options']['tenant_name'])
+
+ # Ensure this is not an endless loop - it fails after the second 401
+ with mock.patch('swiftclient.client.http_connection',
+ self.fake_http_connection(401, 401, 401, 401)):
+ with mock.patch('swiftclient.client.sleep', swap_sleep):
+ self.swap_sleep_called = False
+
+ conn = c.Connection('http://www.test.com', 'asdf', 'asdf',
+ preauthurl='http://www.old.com',
+ preauthtoken='old',
+ os_options=self.os_options,
+ retries=0)
+
+ self.assertEqual(conn.attempts, 0)
+ self.assertRaises(c.ClientException, conn.head_account)
+ self.assertEqual(conn.attempts, 2)
+ unused_responses = list(self.fake_connect.code_iter)
+ self.assertEqual(unused_responses, [401, 401])
+
def test_service_token_get_account(self):
with mock.patch('swiftclient.client.http_connection',
self.fake_http_connection(200)):