From 025da14326d29051119fff50424748e41bf4be42 Mon Sep 17 00:00:00 2001
From: Daniel Wakefield <daniel.wakefield@hp.com>
Date: Mon, 20 Oct 2014 18:00:01 +0100
Subject: [PATCH] Replaces Stacktraces with useful error messages.
Changed the message shown when a user doesn't enter project/tenant
id/name to be more informative.
When attempting to stat a container without supplying project/tenant
name or id, an empty response was being returned instead of an error
being raised.
Changed the error raised in swiftclient.client when no tenant or project
is specified to be more specific.
Add tests for basic regression checking.
Closes-Bug: #1372589
Change-Id: I4eb6964d9f1702db119cc0294edc02841b1ecd5f
---
swiftclient/client.py | 4 ++-
swiftclient/service.py | 2 +-
swiftclient/shell.py | 26 ++++++++------
tests/unit/test_shell.py | 73 +++++++++++++++++++++++++++++++++++++---
4 files changed, 88 insertions(+), 17 deletions(-)
diff --git a/swiftclient/client.py b/swiftclient/client.py
index def40bf4..8cbedc74 100644
--- a/swiftclient/client.py
+++ b/swiftclient/client.py
@@ -396,7 +396,9 @@ def get_auth(auth_url, user, key, **kwargs):
if not (os_options.get('tenant_name') or os_options.get('tenant_id')
or os_options.get('project_name')
or os_options.get('project_id')):
- raise ClientException('No tenant specified')
+ if auth_version in AUTH_VERSIONS_V2:
+ raise ClientException('No tenant specified')
+ raise ClientException('No project name or project id specified.')
cacert = kwargs.get('cacert', None)
storage_url, token = get_auth_keystone(auth_url, user,
diff --git a/swiftclient/service.py b/swiftclient/service.py
index a91c6050..15decf7f 100644
--- a/swiftclient/service.py
+++ b/swiftclient/service.py
@@ -434,7 +434,7 @@ class SwiftService(object):
})
return res
except ClientException as err:
- if err.http_status == 404:
+ if err.http_status != 404:
res.update({
'success': False,
'error': err
diff --git a/swiftclient/shell.py b/swiftclient/shell.py
index 619ec5c6..015c9d9f 100755
--- a/swiftclient/shell.py
+++ b/swiftclient/shell.py
@@ -488,13 +488,15 @@ def st_stat(parser, args, output_manager):
_opts = vars(options)
with SwiftService(options=_opts) as swift:
- if not args:
- stat_result = swift.stat()
- items = stat_result['items']
- headers = stat_result['headers']
- print_account_stats(items, headers, output_manager)
- else:
- try:
+ try:
+ if not args:
+ stat_result = swift.stat()
+ if not stat_result['success']:
+ raise stat_result['error']
+ items = stat_result['items']
+ headers = stat_result['headers']
+ print_account_stats(items, headers, output_manager)
+ else:
container = args[0]
if '/' in container:
output_manager.error(
@@ -505,6 +507,8 @@ def st_stat(parser, args, output_manager):
args = args[1:]
if not args:
stat_result = swift.stat(container=container)
+ if not stat_result['success']:
+ raise stat_result['error']
items = stat_result['items']
headers = stat_result['headers']
print_container_stats(items, headers, output_manager)
@@ -527,8 +531,8 @@ def st_stat(parser, args, output_manager):
'Usage: %s stat %s\n%s', BASENAME,
st_stat_options, st_stat_help)
- except SwiftError as e:
- output_manager.error(e.value)
+ except SwiftError as e:
+ output_manager.error(e.value)
st_post_options = '''[--read-acl <acl>] [--write-acl <acl>] [--sync-to]
@@ -961,8 +965,8 @@ def parse_args(parser, args, enforce_requires=True):
if (not (options.auth and options.user and options.key)
and options.auth_version != '3'):
- # Use keystone auth if any of the old-style args are missing
- options.auth_version = '2.0'
+ # Use keystone auth if any of the old-style args are missing
+ options.auth_version = '2.0'
# Use new-style args if old ones not present
if not options.auth and options.os_auth_url:
diff --git a/tests/unit/test_shell.py b/tests/unit/test_shell.py
index 8e61cb42..29d74729 100644
--- a/tests/unit/test_shell.py
+++ b/tests/unit/test_shell.py
@@ -24,6 +24,7 @@ import swiftclient
from swiftclient.service import SwiftError
import swiftclient.shell
import swiftclient.utils
+from swiftclient.multithreading import OutputManager
from os.path import basename, dirname
from tests.unit.test_swiftclient import MockHttpTest
@@ -551,13 +552,13 @@ class TestParsing(unittest.TestCase):
# check the expected opts are set
for key, v in opts.items():
actual = getattr(actual_opts, key)
- self.assertEqual(v, actual, 'Expected %s for key %s, found %s'
- % (v, key, actual))
+ self.assertEqual(v, actual, 'Expected %s for key %s, found %s' %
+ (v, key, actual))
for key, v in os_opts.items():
actual = getattr(actual_opts, "os_" + key)
- self.assertEqual(v, actual, 'Expected %s for key %s, found %s'
- % (v, key, actual))
+ self.assertEqual(v, actual, 'Expected %s for key %s, found %s' %
+ (v, key, actual))
# check the os_options dict values are set
self.assertTrue(hasattr(actual_opts, 'os_options'))
@@ -725,6 +726,55 @@ class TestParsing(unittest.TestCase):
args = _make_args("stat", opts, os_opts)
self.assertRaises(SystemExit, swiftclient.shell.main, args)
+ def test_no_tenant_name_or_id_v2(self):
+ os_opts = {"password": "secret",
+ "username": "user",
+ "auth_url": "http://example.com:5000/v3",
+ "tenant_name": "",
+ "tenant_id": ""}
+
+ out = six.StringIO()
+ err = six.StringIO()
+ mock_output = _make_output_manager(out, err)
+ with mock.patch('swiftclient.shell.OutputManager', mock_output):
+ args = _make_args("stat", {}, os_opts)
+ self.assertRaises(SystemExit, swiftclient.shell.main, args)
+ self.assertEqual(err.getvalue().strip(), 'No tenant specified')
+
+ out = six.StringIO()
+ err = six.StringIO()
+ mock_output = _make_output_manager(out, err)
+ with mock.patch('swiftclient.shell.OutputManager', mock_output):
+ args = _make_args("stat", {}, os_opts, cmd_args=["testcontainer"])
+ self.assertRaises(SystemExit, swiftclient.shell.main, args)
+ self.assertEqual(err.getvalue().strip(), 'No tenant specified')
+
+ def test_no_tenant_name_or_id_v3(self):
+ os_opts = {"password": "secret",
+ "username": "user",
+ "auth_url": "http://example.com:5000/v3",
+ "tenant_name": "",
+ "tenant_id": ""}
+
+ out = six.StringIO()
+ err = six.StringIO()
+ mock_output = _make_output_manager(out, err)
+ with mock.patch('swiftclient.shell.OutputManager', mock_output):
+ args = _make_args("stat", {"auth_version": "3"}, os_opts)
+ self.assertRaises(SystemExit, swiftclient.shell.main, args)
+ self.assertEqual(err.getvalue().strip(),
+ 'No project name or project id specified.')
+
+ out = six.StringIO()
+ err = six.StringIO()
+ mock_output = _make_output_manager(out, err)
+ with mock.patch('swiftclient.shell.OutputManager', mock_output):
+ args = _make_args("stat", {"auth_version": "3"},
+ os_opts, cmd_args=["testcontainer"])
+ self.assertRaises(SystemExit, swiftclient.shell.main, args)
+ self.assertEqual(err.getvalue().strip(),
+ 'No project name or project id specified.')
+
def test_insufficient_env_vars_v3(self):
args = _make_args("stat", {}, {})
opts = {"auth_version": "3"}
@@ -1043,3 +1093,18 @@ class TestKeystoneOptions(MockHttpTest):
opts = {'auth-version': '2.0'}
self._test_options(opts, os_opts)
+
+
+def _make_output_manager(stdout, stderr):
+ class MockOutputManager(OutputManager):
+ # This class is used to mock OutputManager so that we can
+ # override stdout and stderr. Mocking sys.stdout & sys.stdout
+ # doesn't work because they are argument defaults in the
+ # OutputManager constructor and those defaults are pinned to
+ # the value of sys.stdout/stderr before we get chance to mock them.
+ def __init__(self, print_stream=None, error_stream=None):
+ super(MockOutputManager, self).__init__()
+ self.print_stream = stdout
+ self.error_stream = stderr
+
+ return MockOutputManager