diff options
| -rw-r--r-- | swiftclient/client.py | 10 | ||||
| -rw-r--r-- | swiftclient/service.py | 98 | ||||
| -rwxr-xr-x | swiftclient/shell.py | 13 | ||||
| -rw-r--r-- | test/functional/test_swiftclient.py | 6 | ||||
| -rw-r--r-- | test/unit/test_shell.py | 42 | ||||
| -rw-r--r-- | test/unit/test_swiftclient.py | 53 |
6 files changed, 143 insertions, 79 deletions
diff --git a/swiftclient/client.py b/swiftclient/client.py index 168bfed..e42ac70 100644 --- a/swiftclient/client.py +++ b/swiftclient/client.py @@ -1646,7 +1646,7 @@ class Connection: starting_backoff=1, max_backoff=64, tenant_name=None, os_options=None, auth_version="1", cacert=None, insecure=False, cert=None, cert_key=None, - ssl_compression=True, retry_on_ratelimit=False, + ssl_compression=True, retry_on_ratelimit=True, timeout=None, session=None, force_auth_retry=False): """ :param authurl: authentication URL @@ -1678,9 +1678,9 @@ class Connection: will be made. This may provide a performance increase for https upload/download operations. :param retry_on_ratelimit: by default, a ratelimited connection will - raise an exception to the caller. Setting - this parameter to True will cause a retry - after a backoff. + retry after a backoff. Setting this + parameter to False will cause an exception + to be raised to the caller. :param timeout: The connect timeout for the HTTP connection. :param session: A keystoneauth session object. :param force_auth_retry: reset auth info even if client got unexpected @@ -1825,7 +1825,7 @@ class Connection: self.http_conn = None elif 500 <= err.http_status <= 599: pass - elif self.retry_on_ratelimit and err.http_status == 498: + elif self.retry_on_ratelimit and err.http_status in (498, 429): pass else: raise diff --git a/swiftclient/service.py b/swiftclient/service.py index ed0f40a..9a6c7a1 100644 --- a/swiftclient/service.py +++ b/swiftclient/service.py @@ -155,6 +155,7 @@ def _build_default_global_options(): "user": environ.get('ST_USER'), "key": environ.get('ST_KEY'), "retries": 5, + "retry_on_ratelimit": True, "force_auth_retry": False, "os_username": environ.get('OS_USERNAME'), "os_user_id": environ.get('OS_USER_ID'), @@ -201,6 +202,7 @@ _default_local_options = { 'leave_segments': False, 'changed': None, 'skip_identical': False, + 'skip_container_put': False, 'version_id': None, 'yes_all': False, 'read_acl': None, @@ -270,10 +272,12 @@ def get_conn(options): """ Return a connection building it from the options. """ + options = dict(_default_global_options, **options) return Connection(options['auth'], options['user'], options['key'], timeout=options.get('timeout'), + retry_on_ratelimit=options['retry_on_ratelimit'], retries=options['retries'], auth_version=options['auth_version'], os_options=options['os_options'], @@ -1462,6 +1466,7 @@ class SwiftService: 'leave_segments': False, 'changed': None, 'skip_identical': False, + 'skip_container_put': False, 'fail_fast': False, 'dir_marker': False # Only for None sources } @@ -1487,54 +1492,57 @@ class SwiftService: # the object name. (same as passing --object-name). container, _sep, pseudo_folder = container.partition('/') - # Try to create the container, just in case it doesn't exist. If this - # fails, it might just be because the user doesn't have container PUT - # permissions, so we'll ignore any error. If there's really a problem, - # it'll surface on the first object PUT. - policy_header = {} - _header = split_headers(options["header"]) - if POLICY in _header: - policy_header[POLICY] = \ - _header[POLICY] - create_containers = [ - self.thread_manager.container_pool.submit( - self._create_container_job, container, headers=policy_header) - ] + if not options['skip_container_put']: + # Try to create the container, just in case it doesn't exist. If + # this fails, it might just be because the user doesn't have + # container PUT permissions, so we'll ignore any error. If there's + # really a problem, it'll surface on the first object PUT. + policy_header = {} + _header = split_headers(options["header"]) + if POLICY in _header: + policy_header[POLICY] = \ + _header[POLICY] + create_containers = [ + self.thread_manager.container_pool.submit( + self._create_container_job, container, + headers=policy_header) + ] - # wait for first container job to complete before possibly attempting - # segment container job because segment container job may attempt - # to HEAD the first container - for r in interruptable_as_completed(create_containers): - res = r.result() - yield res + # wait for first container job to complete before possibly + # attempting segment container job because segment container job + # may attempt to HEAD the first container + for r in interruptable_as_completed(create_containers): + res = r.result() + yield res - if segment_size: - seg_container = container + '_segments' - if options['segment_container']: - seg_container = options['segment_container'] - if seg_container != container: - if not policy_header: - # Since no storage policy was specified on the command - # line, rather than just letting swift pick the default - # storage policy, we'll try to create the segments - # container with the same policy as the upload container - create_containers = [ - self.thread_manager.container_pool.submit( - self._create_container_job, seg_container, - policy_source=container - ) - ] - else: - create_containers = [ - self.thread_manager.container_pool.submit( - self._create_container_job, seg_container, - headers=policy_header - ) - ] + if segment_size: + seg_container = container + '_segments' + if options['segment_container']: + seg_container = options['segment_container'] + if seg_container != container: + if not policy_header: + # Since no storage policy was specified on the command + # line, rather than just letting swift pick the default + # storage policy, we'll try to create the segments + # container with the same policy as the upload + # container + create_containers = [ + self.thread_manager.container_pool.submit( + self._create_container_job, seg_container, + policy_source=container + ) + ] + else: + create_containers = [ + self.thread_manager.container_pool.submit( + self._create_container_job, seg_container, + headers=policy_header + ) + ] - for r in interruptable_as_completed(create_containers): - res = r.result() - yield res + for r in interruptable_as_completed(create_containers): + res = r.result() + yield res # We maintain a results queue here and a separate thread to monitor # the futures because we want to get results back from potential diff --git a/swiftclient/shell.py b/swiftclient/shell.py index ed39b81..882a1c0 100755 --- a/swiftclient/shell.py +++ b/swiftclient/shell.py @@ -978,8 +978,9 @@ def st_copy(parser, args, output_manager, return_parser=False): st_upload_options = '''[--changed] [--skip-identical] [--segment-size <size>] [--segment-container <container>] [--leave-segments] [--object-threads <thread>] [--segment-threads <threads>] - [--meta <name:value>] [--header <header>] [--use-slo] - [--ignore-checksum] [--object-name <object-name>] + [--meta <name:value>] [--header <header>] + [--use-slo] [--ignore-checksum] [--skip-container-put] + [--object-name <object-name>] <container> <file_or_directory> [<file_or_directory>] [...] ''' @@ -1025,11 +1026,13 @@ Optional arguments: --use-slo When used in conjunction with --segment-size it will create a Static Large Object instead of the default Dynamic Large Object. + --ignore-checksum Turn off checksum validation for uploads. + --skip-container-put Assume all necessary containers already exist; don't + automatically try to create them. --object-name <object-name> Upload file and name object to <object-name> or upload dir and use <object-name> as object prefix instead of folder name. - --ignore-checksum Turn off checksum validation for uploads. '''.strip('\n') @@ -1045,6 +1048,10 @@ def st_upload(parser, args, output_manager, return_parser=False): default=False, help='Skip uploading files that are identical on ' 'both sides.') parser.add_argument( + '--skip-container-put', action='store_true', dest='skip_container_put', + default=False, help='Assume all necessary containers already exist; ' + "don't automatically try to create them.") + parser.add_argument( '-S', '--segment-size', dest='segment_size', help='Upload files ' 'in segments no larger than <size> (in Bytes) and then create a ' '"manifest" file that will download all the segments as if it were ' diff --git a/test/functional/test_swiftclient.py b/test/functional/test_swiftclient.py index 91e31af..a5c1211 100644 --- a/test/functional/test_swiftclient.py +++ b/test/functional/test_swiftclient.py @@ -332,7 +332,7 @@ class TestFunctional(unittest.TestCase): resp_chunk_size=resp_chunk_size) data = next(body) self.assertEqual(self.test_data[:resp_chunk_size], data) - self.assertTrue(1, self.conn.attempts) + self.assertEqual(1, self.conn.attempts) for chunk in body.resp: # Flush remaining data from underlying response # (simulate a dropped connection) @@ -369,13 +369,13 @@ class TestFunctional(unittest.TestCase): hdrs, body = self.conn.get_object(self.containername, self.objectname) data = body self.assertEqual(self.test_data, data) - self.assertTrue(1, self.conn.attempts) + self.assertEqual(1, self.conn.attempts) hdrs, body = self.conn.get_object(self.containername, self.objectname, resp_chunk_size=0) data = body self.assertEqual(self.test_data, data) - self.assertTrue(1, self.conn.attempts) + self.assertEqual(1, self.conn.attempts) def test_post_account(self): self.conn.post_account({'x-account-meta-data': 'Something'}) diff --git a/test/unit/test_shell.py b/test/unit/test_shell.py index 80c031e..98d73e9 100644 --- a/test/unit/test_shell.py +++ b/test/unit/test_shell.py @@ -907,6 +907,48 @@ class TestShell(unittest.TestCase): query_string='multipart-manifest=put', response_dict=mock.ANY) + @mock.patch('swiftclient.shell.walk') + @mock.patch('swiftclient.service.Connection') + def test_upload_skip_container_put(self, connection, walk): + connection.return_value.head_object.return_value = { + 'content-length': '0'} + connection.return_value.put_object.return_value = EMPTY_ETAG + connection.return_value.attempts = 0 + argv = ["", "upload", "container", "--skip-container-put", + self.tmpfile, "-H", "X-Storage-Policy:one", + "--meta", "Color:Blue"] + swiftclient.shell.main(argv) + connection.return_value.put_container.assert_not_called() + + connection.return_value.put_object.assert_called_with( + 'container', + self.tmpfile.lstrip('/'), + mock.ANY, + content_length=0, + headers={'x-object-meta-mtime': mock.ANY, + 'X-Storage-Policy': 'one', + 'X-Object-Meta-Color': 'Blue'}, + response_dict={}) + + # Upload in segments + connection.return_value.head_container.return_value = { + 'x-storage-policy': 'one'} + argv = ["", "upload", "container", "--skip-container-put", + self.tmpfile, "-S", "10"] + with open(self.tmpfile, "wb") as fh: + fh.write(b'12345678901234567890') + swiftclient.shell.main(argv) + # Both base and segments container are assumed to exist already + connection.return_value.put_container.assert_not_called() + connection.return_value.put_object.assert_called_with( + 'container', + self.tmpfile.lstrip('/'), + '', + content_length=0, + headers={'x-object-manifest': mock.ANY, + 'x-object-meta-mtime': mock.ANY}, + response_dict={}) + @mock.patch('swiftclient.service.SwiftService.upload') def test_upload_object_with_account_readonly(self, upload): argv = ["", "upload", "container", self.tmpfile] diff --git a/test/unit/test_swiftclient.py b/test/unit/test_swiftclient.py index ad2af50..ae3e76f 100644 --- a/test/unit/test_swiftclient.py +++ b/test/unit/test_swiftclient.py @@ -2130,30 +2130,37 @@ class TestConnection(MockHttpTest): pass c.sleep = quick_sleep - # test retries - conn = c.Connection('http://www.test.com/auth/v1.0', 'asdf', 'asdf', - retry_on_ratelimit=True) - code_iter = [200] + [498] * (conn.retries + 1) - auth_resp_headers = { - 'x-auth-token': 'asdf', - 'x-storage-url': 'http://storage/v1/test', - } - c.http_connection = self.fake_http_connection( - *code_iter, headers=auth_resp_headers) - with self.assertRaises(c.ClientException) as exc_context: - conn.head_account() - self.assertIn('Account HEAD failed', str(exc_context.exception)) - self.assertEqual(conn.attempts, conn.retries + 1) + def test_status_code(code): + # test retries + conn = c.Connection('http://www.test.com/auth/v1.0', + 'asdf', 'asdf', retry_on_ratelimit=True) + code_iter = [200] + [code] * (conn.retries + 1) + auth_resp_headers = { + 'x-auth-token': 'asdf', + 'x-storage-url': 'http://storage/v1/test', + } + c.http_connection = self.fake_http_connection( + *code_iter, headers=auth_resp_headers) + with self.assertRaises(c.ClientException) as exc_context: + conn.head_account() + self.assertIn('Account HEAD failed', str(exc_context.exception)) + self.assertEqual(code, exc_context.exception.http_status) + self.assertEqual(conn.attempts, conn.retries + 1) - # test default no-retry - c.http_connection = self.fake_http_connection( - 200, 498, - headers=auth_resp_headers) - conn = c.Connection('http://www.test.com/auth/v1.0', 'asdf', 'asdf') - with self.assertRaises(c.ClientException) as exc_context: - conn.head_account() - self.assertIn('Account HEAD failed', str(exc_context.exception)) - self.assertEqual(conn.attempts, 1) + # test default no-retry + c.http_connection = self.fake_http_connection( + 200, code, + headers=auth_resp_headers) + conn = c.Connection('http://www.test.com/auth/v1.0', + 'asdf', 'asdf', retry_on_ratelimit=False) + with self.assertRaises(c.ClientException) as exc_context: + conn.head_account() + self.assertIn('Account HEAD failed', str(exc_context.exception)) + self.assertEqual(code, exc_context.exception.http_status) + self.assertEqual(conn.attempts, 1) + + test_status_code(498) + test_status_code(429) def test_retry_with_socket_error(self): def quick_sleep(*args): |