openstack/python-swiftclient
Code Issues Proposed changes

Make swift post output an error message when failing

Browse Source 
swiftclient.shell.st_post was ignoring the result dict
returned from SwiftService.post and therefore giving no
indication when posts failed e.g. due to invalid auth
credentials.

This patch ensures that SwiftService always returns a
result_dict from post() and then checks the result dict.
On failure the shell now outputs the ClientException
message and exits with error code 1.

Also adds unit tests and cleans up some unnecessary mocked
return values in existing tests.

Closes-Bug: 1383243

Change-Id: I111150eb3d026c8d01c2cac13d3613ca7304e5b9
This commit is contained in:
Alistair Coles 2014-10-20 10:41:34 +01:00
parent 871a3e40f3
commit 95056d1ef4
3 changed files with 134 additions and 78 deletions
swiftclient
service.pyshell.py
tests/unit
test_shell.py

131
swiftclient/service.py

@ -594,44 +594,31 @@ class SwiftService(object):
'error': err, 'error': err,
'response_dict': response_dict 'response_dict': response_dict
}) })
return res return res
else: if not objects:
if not objects: res["action"] = "post_container"
res["action"] = "post_container" response_dict = {}
response_dict = {} headers = split_headers(
headers = split_headers( options['meta'], 'X-Container-Meta-')
options['meta'], 'X-Container-Meta-') headers.update(
headers.update( split_headers(options['header'], ''))
split_headers(options['header'], '')) if options['read_acl'] is not None:
if options['read_acl'] is not None: headers['X-Container-Read'] = options['read_acl']
headers['X-Container-Read'] = options['read_acl'] if options['write_acl'] is not None:
if options['write_acl'] is not None: headers['X-Container-Write'] = options['write_acl']
headers['X-Container-Write'] = options['write_acl'] if options['sync_to'] is not None:
if options['sync_to'] is not None: headers['X-Container-Sync-To'] = options['sync_to']
headers['X-Container-Sync-To'] = options['sync_to'] if options['sync_key'] is not None:
if options['sync_key'] is not None: headers['X-Container-Sync-Key'] = options['sync_key']
headers['X-Container-Sync-Key'] = options['sync_key'] res['headers'] = headers
res['headers'] = headers try:
try: post = self.thread_manager.container_pool.submit(
post = self.thread_manager.container_pool.submit( self._post_container_job, container,
self._post_container_job, container, headers, response_dict
headers, response_dict )
) get_future_result(post)
get_future_result(post) except ClientException as err:
except ClientException as err: if err.http_status != 404:
if err.http_status != 404:
res.update({
'action': 'post_container',
'success': False,
'error': err,
'response_dict': response_dict
})
return res
raise SwiftError(
"Container '%s' not found" % container,
container=container
)
except Exception as err:
res.update({ res.update({
'action': 'post_container', 'action': 'post_container',
'success': False, 'success': False,
@ -639,37 +626,49 @@ class SwiftService(object):
'response_dict': response_dict 'response_dict': response_dict
}) })
return res return res
else: raise SwiftError(
post_futures = [] "Container '%s' not found" % container,
post_objects = self._make_post_objects(objects) container=container
for post_object in post_objects: )
obj = post_object.object_name except Exception as err:
obj_options = post_object.options res.update({
response_dict = {} 'action': 'post_container',
headers = split_headers( 'success': False,
options['meta'], 'X-Object-Meta-') 'error': err,
# add header options to the headers object for the request. 'response_dict': response_dict
headers.update( })
split_headers(options['header'], '')) return res
if obj_options is not None: else:
if 'meta' in obj_options: post_futures = []
headers.update( post_objects = self._make_post_objects(objects)
split_headers( for post_object in post_objects:
obj_options['meta'], 'X-Object-Meta' obj = post_object.object_name
) obj_options = post_object.options
) response_dict = {}
if 'headers' in obj_options: headers = split_headers(
headers.update( options['meta'], 'X-Object-Meta-')
split_headers(obj_options['header'], '') # add header options to the headers object for the request.
headers.update(
split_headers(options['header'], ''))
if obj_options is not None:
if 'meta' in obj_options:
headers.update(
split_headers(
obj_options['meta'], 'X-Object-Meta'
) )
)
if 'headers' in obj_options:
headers.update(
split_headers(obj_options['header'], '')
)
post = self.thread_manager.object_uu_pool.submit( post = self.thread_manager.object_uu_pool.submit(
self._post_object_job, container, obj, self._post_object_job, container, obj,
headers, response_dict headers, response_dict
) )
post_futures.append(post) post_futures.append(post)
return ResultsIterator(post_futures) return ResultsIterator(post_futures)
@staticmethod @staticmethod
def _make_post_objects(objects): def _make_post_objects(objects):

14
swiftclient/shell.py

@ -543,8 +543,7 @@ If the container is not found, it will be created automatically.
Positional arguments: Positional arguments:
[container] Name of container to post to. [container] Name of container to post to.
[object] Name of object to post. Specify multiple times [object] Name of object to post.
for multiple objects.
Optional arguments: Optional arguments:
--read-acl <acl> Read ACL for containers. Quick summary of ACL syntax: --read-acl <acl> Read ACL for containers. Quick summary of ACL syntax:
@ -596,7 +595,7 @@ def st_post(parser, args, output_manager):
with SwiftService(options=_opts) as swift: with SwiftService(options=_opts) as swift:
try: try:
if not args: if not args:
swift.post() result = swift.post()
else: else:
container = args[0] container = args[0]
if '/' in container: if '/' in container:
@ -612,15 +611,16 @@ def st_post(parser, args, output_manager):
results_iterator = swift.post( results_iterator = swift.post(
container=container, objects=objects container=container, objects=objects
) )
for result in results_iterator: # only 1 result result = next(results_iterator)
if not result["success"]:
raise(result["error"])
else: else:
output_manager.error( output_manager.error(
'Usage: %s post %s\n%s', BASENAME, 'Usage: %s post %s\n%s', BASENAME,
st_post_options, st_post_help) st_post_options, st_post_help)
return
else: else:
swift.post(container=container) result = swift.post(container=container)
if not result["success"]:
raise(result["error"])
except SwiftError as e: except SwiftError as e:
output_manager.error(e.value) output_manager.error(e.value)

67
tests/unit/test_shell.py

@ -386,26 +386,68 @@ class TestShell(unittest.TestCase):
@mock.patch('swiftclient.service.Connection') @mock.patch('swiftclient.service.Connection')
def test_post_account(self, connection): def test_post_account(self, connection):
argv = ["", "post"] argv = ["", "post"]
connection.return_value.head_object.return_value = {}
swiftclient.shell.main(argv) swiftclient.shell.main(argv)
connection.return_value.post_account.assert_called_with( connection.return_value.post_account.assert_called_with(
headers={}, response_dict={}) headers={}, response_dict={})
@mock.patch('swiftclient.shell.OutputManager.error')
@mock.patch('swiftclient.service.Connection')
def test_post_account_bad_auth(self, connection, error):
argv = ["", "post"]
connection.return_value.post_account.side_effect = \
swiftclient.ClientException('bad auth')
swiftclient.shell.main(argv)
error.assert_called_with('bad auth')
@mock.patch('swiftclient.shell.OutputManager.error')
@mock.patch('swiftclient.service.Connection')
def test_post_account_not_found(self, connection, error):
argv = ["", "post"]
connection.return_value.post_account.side_effect = \
swiftclient.ClientException('test', http_status=404)
swiftclient.shell.main(argv)
error.assert_called_with('Account not found')
@mock.patch('swiftclient.service.Connection')
def test_post_container(self, connection):
argv = ["", "post", "container"] argv = ["", "post", "container"]
connection.return_value.head_object.return_value = {}
swiftclient.shell.main(argv) swiftclient.shell.main(argv)
connection.return_value.post_container.assert_called_with( connection.return_value.post_container.assert_called_with(
'container', headers={}, response_dict={}) 'container', headers={}, response_dict={})
@mock.patch('swiftclient.shell.OutputManager.error')
@mock.patch('swiftclient.service.Connection') @mock.patch('swiftclient.service.Connection')
def test_post_container(self, connection): def test_post_container_bad_auth(self, connection, error):
argv = ["", "post", "container"]
connection.return_value.post_container.side_effect = \
swiftclient.ClientException('bad auth')
swiftclient.shell.main(argv)
error.assert_called_with('bad auth')
@mock.patch('swiftclient.service.Connection')
def test_post_container_not_found_causes_put(self, connection):
argv = ["", "post", "container"]
connection.return_value.post_container.side_effect = \
swiftclient.ClientException('test', http_status=404)
swiftclient.shell.main(argv)
self.assertEqual('container',
connection.return_value.put_container.call_args[0][0])
@mock.patch('swiftclient.shell.OutputManager.error')
def test_post_container_with_bad_name(self, error):
argv = ["", "post", "conta/iner"]
swiftclient.shell.main(argv)
self.assertTrue(error.called)
self.assertTrue(error.call_args[0][0].startswith('WARNING: / in'))
@mock.patch('swiftclient.service.Connection')
def test_post_container_with_options(self, connection):
argv = ["", "post", "container", argv = ["", "post", "container",
"--read-acl", "test2:tester2", "--read-acl", "test2:tester2",
"--write-acl", "test3:tester3 test4", "--write-acl", "test3:tester3 test4",
"--sync-to", "othersite", "--sync-to", "othersite",
"--sync-key", "secret", "--sync-key", "secret",
] ]
connection.return_value.head_object.return_value = {}
swiftclient.shell.main(argv) swiftclient.shell.main(argv)
connection.return_value.post_container.assert_called_with( connection.return_value.post_container.assert_called_with(
'container', headers={ 'container', headers={
@ -420,13 +462,28 @@ class TestShell(unittest.TestCase):
"--meta", "Color:Blue", "--meta", "Color:Blue",
"--header", "content-type:text/plain" "--header", "content-type:text/plain"
] ]
connection.return_value.head_object.return_value = {}
swiftclient.shell.main(argv) swiftclient.shell.main(argv)
connection.return_value.post_object.assert_called_with( connection.return_value.post_object.assert_called_with(
'container', 'object', headers={ 'container', 'object', headers={
'Content-Type': 'text/plain', 'Content-Type': 'text/plain',
'X-Object-Meta-Color': 'Blue'}, response_dict={}) 'X-Object-Meta-Color': 'Blue'}, response_dict={})
@mock.patch('swiftclient.shell.OutputManager.error')
@mock.patch('swiftclient.service.Connection')
def test_post_object_bad_auth(self, connection, error):
argv = ["", "post", "container", "object"]
connection.return_value.post_object.side_effect = \
swiftclient.ClientException("bad auth")
swiftclient.shell.main(argv)
error.assert_called_with('bad auth')
@mock.patch('swiftclient.shell.OutputManager.error')
def test_post_object_too_many_args(self, error):
argv = ["", "post", "container", "object", "bad_arg"]
swiftclient.shell.main(argv)
self.assertTrue(error.called)
self.assertTrue(error.call_args[0][0].startswith('Usage'))
@mock.patch('swiftclient.shell.generate_temp_url') @mock.patch('swiftclient.shell.generate_temp_url')
def test_temp_url(self, temp_url): def test_temp_url(self, temp_url):
argv = ["", "tempurl", "GET", "60", "/v1/AUTH_account/c/o", argv = ["", "tempurl", "GET", "60", "/v1/AUTH_account/c/o",