From 23c7a58f8f1412c28b3a16b79be09c224c9f7d55 Mon Sep 17 00:00:00 2001 From: Hisashi Osanai Date: Fri, 11 Dec 2015 18:26:34 +0900 Subject: [PATCH] Fix ClientException handling in Container Sync swift/container/sync.py uses swift.common.internal_client.delete_object and put_object and expected these methods raise ClientException. But delete_object and put_object never raise the exception so this patch raises ClientException when urllib2 library raises HTTPError. Co-Authored-By: Eran Rom Closes-Bug: #1419901 Change-Id: I58cbf77988979a07998a46d9d81be84d29b0d9bf --- bin/swift-dispersion-report | 5 ++--- swift/common/internal_client.py | 12 +++++++++--- test/unit/common/test_internal_client.py | 9 ++++----- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/bin/swift-dispersion-report b/bin/swift-dispersion-report index a1b5fdaab0..48dff80a89 100755 --- a/bin/swift-dispersion-report +++ b/bin/swift-dispersion-report @@ -23,7 +23,6 @@ from time import time from eventlet import GreenPool, hubs, patcher, Timeout from eventlet.pools import Pool -from eventlet.green import urllib2 from swift.common import direct_client try: @@ -174,8 +173,8 @@ def object_dispersion_report(coropool, connpool, account, object_ring, try: objects = [o['name'] for o in conn.get_container( container, prefix='dispersion_', full_listing=True)[1]] - except urllib2.HTTPError as err: - if err.getcode() != 404: + except ClientException as err: + if err.http_status != 404: raise print >>stderr, 'No objects to query. Has ' \ diff --git a/swift/common/internal_client.py b/swift/common/internal_client.py index 7dceda8427..8aaff72eb6 100644 --- a/swift/common/internal_client.py +++ b/swift/common/internal_client.py @@ -26,9 +26,10 @@ from swift import gettext_ as _ from time import gmtime, strftime, time from zlib import compressobj -from swift.common.utils import quote +from swift.common.exceptions import ClientException from swift.common.http import HTTP_NOT_FOUND, HTTP_MULTIPLE_CHOICES from swift.common.swob import Request +from swift.common.utils import quote from swift.common.wsgi import loadapp, pipeline_property @@ -807,9 +808,14 @@ class SimpleClient(object): self.attempts += 1 try: return self.base_request(method, **kwargs) - except (socket.error, httplib.HTTPException, urllib2.URLError): + except (socket.error, httplib.HTTPException, urllib2.URLError) \ + as err: if self.attempts > retries: - raise + if isinstance(err, urllib2.HTTPError): + raise ClientException('Raise too many retries', + http_status=err.getcode()) + else: + raise sleep(backoff) backoff = min(backoff * 2, self.max_backoff) diff --git a/test/unit/common/test_internal_client.py b/test/unit/common/test_internal_client.py index d2ef735324..1dfc31b409 100644 --- a/test/unit/common/test_internal_client.py +++ b/test/unit/common/test_internal_client.py @@ -26,8 +26,7 @@ from six.moves import range from six.moves.urllib.parse import quote from test.unit import FakeLogger from eventlet.green import urllib2 -from swift.common import internal_client -from swift.common import swob +from swift.common import exceptions, internal_client, swob from swift.common.storage_policy import StoragePolicy from test.unit import with_tempdir, write_fake_ring, patch_policies @@ -1329,7 +1328,7 @@ class TestSimpleClient(unittest.TestCase): mock_urlopen.side_effect = urllib2.HTTPError(*[None] * 5) with mock.patch('swift.common.internal_client.sleep') \ as mock_sleep: - self.assertRaises(urllib2.HTTPError, + self.assertRaises(exceptions.ClientException, c.retry_request, request_method, retries=1) self.assertEqual(mock_sleep.call_count, 1) self.assertEqual(mock_urlopen.call_count, 2) @@ -1347,7 +1346,7 @@ class TestSimpleClient(unittest.TestCase): mock_urlopen.side_effect = urllib2.HTTPError(*[None] * 5) with mock.patch('swift.common.internal_client.sleep') \ as mock_sleep: - self.assertRaises(urllib2.HTTPError, + self.assertRaises(exceptions.ClientException, c.retry_request, request_method, container='con', retries=1) self.assertEqual(mock_sleep.call_count, 1) @@ -1366,7 +1365,7 @@ class TestSimpleClient(unittest.TestCase): mock_urlopen.side_effect = urllib2.HTTPError(*[None] * 5) with mock.patch('swift.common.internal_client.sleep') \ as mock_sleep: - self.assertRaises(urllib2.HTTPError, + self.assertRaises(exceptions.ClientException, c.retry_request, request_method, container='con', name='obj', retries=1) self.assertEqual(mock_sleep.call_count, 1)