diff options
author | Stuart McLaren <stuart.mclaren@hp.com> | 2015-04-07 12:32:36 +0000 |
---|---|---|
committer | Stuart McLaren <stuart.mclaren@hp.com> | 2015-05-08 12:51:02 +0000 |
commit | 1f89beb6098f4f6a8d8c2912392b273bc068b2e3 (patch) | |
tree | 058e9fe2b0899aaa3b041df7b6129444e885e4a2 | |
parent | 9fac2a557ee14b597ac52144a0d35c0854024f01 (diff) | |
download | python-glanceclient-1f89beb6098f4f6a8d8c2912392b273bc068b2e3.tar.gz |
Improve import related error handling
If there was a problem importing a library we would incorrectly raise
an unsupported version error:
$ glance --os-image-api-version 1 image-list
"1" is not a supported API version. Example values are "1" or "2".
We should change this to provide information on the failed import, eg:
$ glance --os-image-api-version 1 image-list
No module named badimport
We also now raise the full stacktrace in this case if '--debug' is passed
on the command line.
Change-Id: I1c687ae6c5da239090b0b7a4a855b3271a9076da
Related-bug: 1402632
-rw-r--r-- | glanceclient/shell.py | 33 | ||||
-rw-r--r-- | glanceclient/tests/unit/test_shell.py | 59 |
2 files changed, 80 insertions, 12 deletions
diff --git a/glanceclient/shell.py b/glanceclient/shell.py index f212323..4607b54 100644 --- a/glanceclient/shell.py +++ b/glanceclient/shell.py @@ -47,6 +47,8 @@ from keystoneclient import session osprofiler_profiler = importutils.try_import("osprofiler.profiler") _ = _i18n._ +SUPPORTED_VERSIONS = [1, 2] + class OpenStackImagesShell(object): @@ -291,12 +293,7 @@ class OpenStackImagesShell(object): self.subcommands = {} subparsers = parser.add_subparsers(metavar='<subcommand>') - try: - submodule = utils.import_versioned_module(version, 'shell') - except ImportError: - print('"%s" is not a supported API version. Example ' - 'values are "1" or "2".' % version) - utils.exit() + submodule = utils.import_versioned_module(version, 'shell') self._find_actions(subparsers, submodule) self._find_actions(subparsers, self) @@ -602,14 +599,32 @@ class OpenStackImagesShell(object): # build available subcommands based on version try: api_version = int(options.os_image_api_version or url_version or 1) + if api_version not in SUPPORTED_VERSIONS: + raise ValueError except ValueError: - print("Invalid API version parameter") - utils.exit() + msg = ("Invalid API version parameter. " + "Supported values are %s" % SUPPORTED_VERSIONS) + utils.exit(msg=msg) if api_version == 2: self._cache_schemas(options) - subcommand_parser = self.get_subcommand_parser(api_version) + try: + subcommand_parser = self.get_subcommand_parser(api_version) + except ImportError as e: + if options.debug: + traceback.print_exc() + if not str(e): + # Add a generic import error message if the raised ImportError + # has none. + raise ImportError('Unable to import module. Re-run ' + 'with --debug for more info.') + raise + except Exception: + if options.debug: + traceback.print_exc() + raise + self.parser = subcommand_parser # Handle top-level --help/-h before attempting to parse diff --git a/glanceclient/tests/unit/test_shell.py b/glanceclient/tests/unit/test_shell.py index ef15561..d6ce9e6 100644 --- a/glanceclient/tests/unit/test_shell.py +++ b/glanceclient/tests/unit/test_shell.py @@ -27,10 +27,11 @@ import requests from requests_mock.contrib import fixture as rm_fixture import six +from glanceclient.common import utils from glanceclient import exc from glanceclient import shell as openstack_shell +from glanceclient.tests import utils as testutils -from glanceclient.tests import utils #NOTE (esheffield) Used for the schema caching tests from glanceclient.v2 import schemas as schemas import json @@ -75,7 +76,7 @@ _s = V3_TOKEN.add_service('image', name='glance') _s.add_standard_endpoints(public=DEFAULT_IMAGE_URL) -class ShellTest(utils.TestCase): +class ShellTest(testutils.TestCase): # auth environment to use auth_env = FAKE_V2_ENV.copy() # expected auth plugin to invoke @@ -311,6 +312,58 @@ class ShellTest(utils.TestCase): except SystemExit as ex: self.assertEqual(130, ex.code) + @mock.patch('glanceclient.common.utils.exit', side_effect=utils.exit) + def test_shell_illegal_version(self, mock_exit): + # Only int versions are allowed on cli + shell = openstack_shell.OpenStackImagesShell() + argstr = '--os-image-api-version 1.1 image-list' + try: + shell.main(argstr.split()) + except SystemExit as ex: + self.assertEqual(1, ex.code) + msg = ("Invalid API version parameter. " + "Supported values are %s" % openstack_shell.SUPPORTED_VERSIONS) + mock_exit.assert_called_with(msg=msg) + + @mock.patch('glanceclient.common.utils.exit', side_effect=utils.exit) + def test_shell_unsupported_version(self, mock_exit): + # Test an integer version which is not supported (-1) + shell = openstack_shell.OpenStackImagesShell() + argstr = '--os-image-api-version -1 image-list' + try: + shell.main(argstr.split()) + except SystemExit as ex: + self.assertEqual(1, ex.code) + msg = ("Invalid API version parameter. " + "Supported values are %s" % openstack_shell.SUPPORTED_VERSIONS) + mock_exit.assert_called_with(msg=msg) + + @mock.patch.object(openstack_shell.OpenStackImagesShell, + 'get_subcommand_parser') + def test_shell_import_error_with_mesage(self, mock_parser): + msg = 'Unable to import module xxx' + mock_parser.side_effect = ImportError('%s' % msg) + shell = openstack_shell.OpenStackImagesShell() + argstr = '--os-image-api-version 2 image-list' + try: + shell.main(argstr.split()) + self.fail('No import error returned') + except ImportError as e: + self.assertEqual(msg, str(e)) + + @mock.patch.object(openstack_shell.OpenStackImagesShell, + 'get_subcommand_parser') + def test_shell_import_error_default_message(self, mock_parser): + mock_parser.side_effect = ImportError + shell = openstack_shell.OpenStackImagesShell() + argstr = '--os-image-api-version 2 image-list' + try: + shell.main(argstr.split()) + self.fail('No import error returned') + except ImportError as e: + msg = 'Unable to import module. Re-run with --debug for more info.' + self.assertEqual(msg, str(e)) + @mock.patch('glanceclient.v1.client.Client') def test_auth_plugin_invocation_without_username_with_v1(self, v1_client): self.make_env(exclude='OS_USERNAME') @@ -419,7 +472,7 @@ class ShellTestWithKeystoneV3Auth(ShellTest): self.assertNotIn(r, stdout.split()) -class ShellCacheSchemaTest(utils.TestCase): +class ShellCacheSchemaTest(testutils.TestCase): def setUp(self): super(ShellCacheSchemaTest, self).setUp() self._mock_client_setup() |