From b76f5944130e29ee1bf3095c966a393c489c05e6 Mon Sep 17 00:00:00 2001 From: Cao Shufeng Date: Sun, 10 Jul 2016 19:08:06 +0800 Subject: [PATCH] Add "start_version" and "end_version" support to argparse Now, "cinder help subcommand" can not show whether an argument is supported for a specific microversion. With this change, developers only need to add a start_version or end_version in the utils.arg wrap, cinderclient will support the microversion for that arguement. @utils.arg( '--foo', start_version='3.1') @utils.arg( '--bar', start_version='3.2', end_version='3.5') def do_some_action(): ...... In previous example, an exception will be raised for such command: $ cinder --os-volume-api-version 3.6 --bar some-ation And only "--foo" will show up for such help command: $ cinder --os-volume-api-version 3.1 help some-ation Change-Id: I74137486992846bbf9fdff53c009851db2356eef Partial-Bug: #1600567 Co-Authored-By: Nate Potter --- cinderclient/exceptions.py | 28 +++ cinderclient/shell.py | 98 ++++++++-- .../tests/unit/fake_actions_module.py | 65 +++++++ cinderclient/tests/unit/test_shell.py | 171 ++++++++++++++++++ cinderclient/tests/unit/v3/test_shell.py | 2 +- cinderclient/v3/shell.py | 4 +- 6 files changed, 354 insertions(+), 14 deletions(-) create mode 100644 cinderclient/tests/unit/fake_actions_module.py diff --git a/cinderclient/exceptions.py b/cinderclient/exceptions.py index 72366c3bb..6d9e70754 100644 --- a/cinderclient/exceptions.py +++ b/cinderclient/exceptions.py @@ -28,6 +28,34 @@ class UnsupportedVersion(Exception): pass +class UnsupportedAttribute(AttributeError): + """Indicates that the user is trying to transmit the argument to a method, + which is not supported by selected version. + """ + + def __init__(self, argument_name, start_version, end_version): + if not start_version.is_null() and not end_version.is_null(): + self.message = ( + "'%(name)s' argument is only allowed for microversions " + "%(start)s - %(end)s." % {"name": argument_name, + "start": start_version.get_string(), + "end": end_version.get_string()}) + elif not start_version.is_null(): + self.message = ( + "'%(name)s' argument is only allowed since microversion " + "%(start)s." % {"name": argument_name, + "start": start_version.get_string()}) + + elif not end_version.is_null(): + self.message = ( + "'%(name)s' argument is not allowed after microversion " + "%(end)s." % {"name": argument_name, + "end": end_version.get_string()}) + + def __str__(self): + return self.message + + class InvalidAPIVersion(Exception): pass diff --git a/cinderclient/shell.py b/cinderclient/shell.py index cb2d496c4..d756c0c48 100644 --- a/cinderclient/shell.py +++ b/cinderclient/shell.py @@ -26,6 +26,7 @@ import logging import sys import requests +import six from cinderclient import api_versions from cinderclient import client @@ -56,6 +57,8 @@ DEFAULT_CINDER_ENDPOINT_TYPE = 'publicURL' V1_SHELL = 'cinderclient.v1.shell' V2_SHELL = 'cinderclient.v2.shell' V3_SHELL = 'cinderclient.v3.shell' +HINT_HELP_MSG = (" [hint: use '--os-volume-api-version' flag to show help " + "message for proper version]") logging.basicConfig() logger = logging.getLogger(__name__) @@ -395,24 +398,26 @@ class OpenStackCinderShell(object): parser.set_defaults(insecure=utils.env('CINDERCLIENT_INSECURE', default=False)) - def get_subcommand_parser(self, version): + def get_subcommand_parser(self, version, do_help=False, input_args=None): parser = self.get_base_parser() self.subcommands = {} subparsers = parser.add_subparsers(metavar='') - if version == '2': + if version.ver_major == 2: actions_module = importutils.import_module(V2_SHELL) - elif version == '3': + elif version.ver_major == 3: actions_module = importutils.import_module(V3_SHELL) else: actions_module = importutils.import_module(V1_SHELL) - self._find_actions(subparsers, actions_module) - self._find_actions(subparsers, self) + self._find_actions(subparsers, actions_module, version, do_help, + input_args) + self._find_actions(subparsers, self, version, do_help, input_args) for extension in self.extensions: - self._find_actions(subparsers, extension.module) + self._find_actions(subparsers, extension.module, version, do_help, + input_args) self._add_bash_completion_subparser(subparsers) @@ -427,18 +432,53 @@ class OpenStackCinderShell(object): self.subcommands['bash_completion'] = subparser subparser.set_defaults(func=self.do_bash_completion) - def _find_actions(self, subparsers, actions_module): + def _build_versioned_help_message(self, start_version, end_version): + if not start_version.is_null() and not end_version.is_null(): + msg = (_(" (Supported by API versions %(start)s - %(end)s)") + % {"start": start_version.get_string(), + "end": end_version.get_string()}) + elif not start_version.is_null(): + msg = (_(" (Supported by API version %(start)s and later)") + % {"start": start_version.get_string()}) + else: + msg = (_(" Supported until API version %(end)s)") + % {"end": end_version.get_string()}) + return six.text_type(msg) + + def _find_actions(self, subparsers, actions_module, version, + do_help, input_args): for attr in (a for a in dir(actions_module) if a.startswith('do_')): # I prefer to be hyphen-separated instead of underscores. command = attr[3:].replace('_', '-') callback = getattr(actions_module, attr) desc = callback.__doc__ or '' - help = desc.strip().split('\n')[0] + action_help = desc.strip().split('\n')[0] + if hasattr(callback, "versioned"): + additional_msg = "" + subs = api_versions.get_substitutions( + utils.get_function_name(callback)) + if do_help: + additional_msg = self._build_versioned_help_message( + subs[0].start_version, subs[-1].end_version) + if version.is_latest(): + additional_msg += HINT_HELP_MSG + subs = [versioned_method for versioned_method in subs + if version.matches(versioned_method.start_version, + versioned_method.end_version)] + if not subs: + # There is no proper versioned method. + continue + # Use the "latest" substitution. + callback = subs[-1].func + desc = callback.__doc__ or desc + action_help = desc.strip().split('\n')[0] + action_help += additional_msg + arguments = getattr(callback, 'arguments', []) subparser = subparsers.add_parser( command, - help=help, + help=action_help, description=desc, add_help=False, formatter_class=OpenStackHelpFormatter) @@ -448,8 +488,40 @@ class OpenStackCinderShell(object): help=argparse.SUPPRESS,) self.subcommands[command] = subparser + + # NOTE(ntpttr): We get a counter for each argument in this + # command here because during the microversion check we only + # want to raise an exception if no version of the argument + # matches the current microversion. The exception will only + # be raised after the last instance of a particular argument + # fails the check. + arg_counter = dict() for (args, kwargs) in arguments: - subparser.add_argument(*args, **kwargs) + arg_counter[args[0]] = arg_counter.get(args[0], 0) + 1 + + for (args, kwargs) in arguments: + start_version = kwargs.get("start_version", None) + start_version = api_versions.APIVersion(start_version) + end_version = kwargs.get('end_version', None) + end_version = api_versions.APIVersion(end_version) + if do_help and not (start_version.is_null() + and end_version.is_null()): + kwargs["help"] = kwargs.get("help", "") + ( + self._build_versioned_help_message(start_version, + end_version)) + if not version.matches(start_version, end_version): + if args[0] in input_args and command == input_args[0]: + if arg_counter[args[0]] == 1: + # This is the last version of this argument, + # raise the exception. + raise exc.UnsupportedAttribute(args[0], + start_version, end_version) + arg_counter[args[0]] -= 1 + continue + kw = kwargs.copy() + kw.pop("start_version", None) + kw.pop("end_version", None) + subparser.add_argument(*args, **kw) subparser.set_defaults(func=callback) def setup_debugging(self, debug): @@ -502,6 +574,9 @@ class OpenStackCinderShell(object): api_version_input = True self.options = options + do_help = ('help' in argv) or ( + '--help' in argv) or ('-h' in argv) or not argv + if not options.os_volume_api_version: api_version = api_versions.get_api_version( DEFAULT_MAJOR_OS_VOLUME_API_VERSION) @@ -514,7 +589,8 @@ class OpenStackCinderShell(object): self.extensions = client.discover_extensions(major_version_string) self._run_extension_hooks('__pre_parse_args__') - subcommand_parser = self.get_subcommand_parser(major_version_string) + subcommand_parser = self.get_subcommand_parser(api_version, + do_help, args) self.parser = subcommand_parser if options.help or not argv: diff --git a/cinderclient/tests/unit/fake_actions_module.py b/cinderclient/tests/unit/fake_actions_module.py new file mode 100644 index 000000000..e5f08f25e --- /dev/null +++ b/cinderclient/tests/unit/fake_actions_module.py @@ -0,0 +1,65 @@ +# Copyright 2016 FUJITSU LIMITED +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from cinderclient import api_versions +from cinderclient import utils + + +@api_versions.wraps("3.0", "3.1") +def do_fake_action(): + """help message + + This will not show up in help message + """ + return "fake_action 3.0 to 3.1" + + +@api_versions.wraps("3.2", "3.3") +def do_fake_action(): + return "fake_action 3.2 to 3.3" + + +@api_versions.wraps("3.6") +@utils.arg( + '--foo', + start_version='3.7') +def do_another_fake_action(): + return "another_fake_action" + + +@utils.arg( + '--foo', + start_version='3.1', + end_version='3.2') +@utils.arg( + '--bar', + help='bar help', + start_version='3.3', + end_version='3.4') +def do_fake_action2(): + return "fake_action2" + + +@utils.arg( + '--foo', + help='first foo', + start_version='3.6', + end_version='3.7') +@utils.arg( + '--foo', + help='second foo', + start_version='3.8') +def do_fake_action3(): + return "fake_action3" diff --git a/cinderclient/tests/unit/test_shell.py b/cinderclient/tests/unit/test_shell.py index c49d9df93..7d6cbaea1 100644 --- a/cinderclient/tests/unit/test_shell.py +++ b/cinderclient/tests/unit/test_shell.py @@ -27,9 +27,11 @@ from six import moves from testtools import matchers import cinderclient +from cinderclient import api_versions from cinderclient import exceptions from cinderclient import auth_plugin from cinderclient import shell +from cinderclient.tests.unit import fake_actions_module from cinderclient.tests.unit.test_auth_plugins import mock_http_request from cinderclient.tests.unit.test_auth_plugins import requested_headers from cinderclient.tests.unit.fixture_data import keystone_client @@ -304,3 +306,172 @@ class CinderClientArgumentParserTest(utils.TestCase): help=argparse.SUPPRESS) self.assertRaises(SystemExit, parser.parse_args, ['--test']) + + +class TestLoadVersionedActions(utils.TestCase): + + def test_load_versioned_actions(self): + parser = cinderclient.shell.CinderClientArgumentParser() + subparsers = parser.add_subparsers(metavar='') + shell = cinderclient.shell.OpenStackCinderShell() + shell.subcommands = {} + shell._find_actions(subparsers, fake_actions_module, + api_versions.APIVersion("3.0"), False, []) + self.assertIn('fake-action', shell.subcommands.keys()) + self.assertEqual( + "fake_action 3.0 to 3.1", + shell.subcommands['fake-action'].get_default('func')()) + + shell.subcommands = {} + shell._find_actions(subparsers, fake_actions_module, + api_versions.APIVersion("3.2"), False, []) + self.assertIn('fake-action', shell.subcommands.keys()) + self.assertEqual( + "fake_action 3.2 to 3.3", + shell.subcommands['fake-action'].get_default('func')()) + + self.assertIn('fake-action2', shell.subcommands.keys()) + self.assertEqual( + "fake_action2", + shell.subcommands['fake-action2'].get_default('func')()) + + def test_load_versioned_actions_not_in_version_range(self): + parser = cinderclient.shell.CinderClientArgumentParser() + subparsers = parser.add_subparsers(metavar='') + shell = cinderclient.shell.OpenStackCinderShell() + shell.subcommands = {} + shell._find_actions(subparsers, fake_actions_module, + api_versions.APIVersion('3.10000'), False, []) + self.assertNotIn('fake-action', shell.subcommands.keys()) + self.assertIn('fake-action2', shell.subcommands.keys()) + + def test_load_versioned_actions_unsupported_input(self): + parser = cinderclient.shell.CinderClientArgumentParser() + subparsers = parser.add_subparsers(metavar='') + shell = cinderclient.shell.OpenStackCinderShell() + shell.subcommands = {} + self.assertRaises(exceptions.UnsupportedAttribute, + shell._find_actions, subparsers, fake_actions_module, + api_versions.APIVersion('3.6'), False, + ['another-fake-action', '--foo']) + + def test_load_versioned_actions_with_help(self): + parser = cinderclient.shell.CinderClientArgumentParser() + subparsers = parser.add_subparsers(metavar='') + shell = cinderclient.shell.OpenStackCinderShell() + shell.subcommands = {} + with mock.patch.object(subparsers, 'add_parser') as mock_add_parser: + shell._find_actions(subparsers, fake_actions_module, + api_versions.APIVersion("3.1"), True, []) + self.assertIn('fake-action', shell.subcommands.keys()) + expected_help = ("help message (Supported by API versions " + "%(start)s - %(end)s)") % { + 'start': '3.0', 'end': '3.3'} + expected_desc = ("help message\n\n " + "This will not show up in help message\n ") + mock_add_parser.assert_any_call( + 'fake-action', + help=expected_help, + description=expected_desc, + add_help=False, + formatter_class=cinderclient.shell.OpenStackHelpFormatter) + + def test_load_versioned_actions_with_help_on_latest(self): + parser = cinderclient.shell.CinderClientArgumentParser() + subparsers = parser.add_subparsers(metavar='') + shell = cinderclient.shell.OpenStackCinderShell() + shell.subcommands = {} + with mock.patch.object(subparsers, 'add_parser') as mock_add_parser: + shell._find_actions(subparsers, fake_actions_module, + api_versions.APIVersion("3.latest"), True, []) + self.assertIn('another-fake-action', shell.subcommands.keys()) + expected_help = (" (Supported by API versions %(start)s - " + "%(end)s)%(hint)s") % { + 'start': '3.6', 'end': '3.latest', + 'hint': cinderclient.shell.HINT_HELP_MSG} + mock_add_parser.assert_any_call( + 'another-fake-action', + help=expected_help, + description='', + add_help=False, + formatter_class=cinderclient.shell.OpenStackHelpFormatter) + + @mock.patch.object(cinderclient.shell.CinderClientArgumentParser, + 'add_argument') + def test_load_versioned_actions_with_args(self, mock_add_arg): + parser = cinderclient.shell.CinderClientArgumentParser(add_help=False) + subparsers = parser.add_subparsers(metavar='') + shell = cinderclient.shell.OpenStackCinderShell() + shell.subcommands = {} + shell._find_actions(subparsers, fake_actions_module, + api_versions.APIVersion("3.1"), False, []) + self.assertIn('fake-action2', shell.subcommands.keys()) + mock_add_arg.assert_has_calls([ + mock.call('-h', '--help', action='help', help='==SUPPRESS=='), + mock.call('--foo')]) + + @mock.patch.object(cinderclient.shell.CinderClientArgumentParser, + 'add_argument') + def test_load_versioned_actions_with_args2(self, mock_add_arg): + parser = cinderclient.shell.CinderClientArgumentParser(add_help=False) + subparsers = parser.add_subparsers(metavar='') + shell = cinderclient.shell.OpenStackCinderShell() + shell.subcommands = {} + shell._find_actions(subparsers, fake_actions_module, + api_versions.APIVersion("3.4"), False, []) + self.assertIn('fake-action2', shell.subcommands.keys()) + mock_add_arg.assert_has_calls([ + mock.call('-h', '--help', action='help', help='==SUPPRESS=='), + mock.call('--bar', help="bar help")]) + + @mock.patch.object(cinderclient.shell.CinderClientArgumentParser, + 'add_argument') + def test_load_versioned_actions_with_args_not_in_version_range( + self, mock_add_arg): + parser = cinderclient.shell.CinderClientArgumentParser(add_help=False) + subparsers = parser.add_subparsers(metavar='') + shell = cinderclient.shell.OpenStackCinderShell() + shell.subcommands = {} + shell._find_actions(subparsers, fake_actions_module, + api_versions.APIVersion("3.10000"), False, []) + self.assertIn('fake-action2', shell.subcommands.keys()) + mock_add_arg.assert_has_calls([ + mock.call('-h', '--help', action='help', help='==SUPPRESS==')]) + + @mock.patch.object(cinderclient.shell.CinderClientArgumentParser, + 'add_argument') + def test_load_versioned_actions_with_args_and_help(self, mock_add_arg): + parser = cinderclient.shell.CinderClientArgumentParser(add_help=False) + subparsers = parser.add_subparsers(metavar='') + shell = cinderclient.shell.OpenStackCinderShell() + shell.subcommands = {} + shell._find_actions(subparsers, fake_actions_module, + api_versions.APIVersion("3.4"), True, []) + mock_add_arg.assert_has_calls([ + mock.call('-h', '--help', action='help', help='==SUPPRESS=='), + mock.call('--bar', + help="bar help (Supported by API versions" + " 3.3 - 3.4)")]) + + @mock.patch.object(cinderclient.shell.CinderClientArgumentParser, + 'add_argument') + def test_load_actions_with_versioned_args(self, mock_add_arg): + parser = cinderclient.shell.CinderClientArgumentParser(add_help=False) + subparsers = parser.add_subparsers(metavar='') + shell = cinderclient.shell.OpenStackCinderShell() + shell.subcommands = {} + shell._find_actions(subparsers, fake_actions_module, + api_versions.APIVersion("3.6"), False, []) + self.assertIn(mock.call('--foo', help="first foo"), + mock_add_arg.call_args_list) + self.assertNotIn(mock.call('--foo', help="second foo"), + mock_add_arg.call_args_list) + + mock_add_arg.reset_mock() + + shell._find_actions(subparsers, fake_actions_module, + api_versions.APIVersion("3.9"), False, []) + self.assertNotIn(mock.call('--foo', help="first foo"), + mock_add_arg.call_args_list) + self.assertIn(mock.call('--foo', help="second foo"), + mock_add_arg.call_args_list) diff --git a/cinderclient/tests/unit/v3/test_shell.py b/cinderclient/tests/unit/v3/test_shell.py index f54ed6606..c56911dbf 100644 --- a/cinderclient/tests/unit/v3/test_shell.py +++ b/cinderclient/tests/unit/v3/test_shell.py @@ -129,7 +129,7 @@ class ShellTest(utils.TestCase): '--os-volume-api-version 3.9 backup-update 1234') def test_backup_update_wrong_version(self): - self.assertRaises(exceptions.VersionNotFoundForAPIMethod, + self.assertRaises(SystemExit, self.run_command, '--os-volume-api-version 3.8 ' 'backup-update --name new-name 1234') diff --git a/cinderclient/v3/shell.py b/cinderclient/v3/shell.py index 9ad688114..145fdf49d 100644 --- a/cinderclient/v3/shell.py +++ b/cinderclient/v3/shell.py @@ -1602,14 +1602,14 @@ def do_backup_reset_state(cs, args): raise exceptions.CommandError(msg) +@utils.service_type('volumev3') +@api_versions.wraps('3.9') @utils.arg('backup', metavar='', help='Name or ID of backup to rename.') @utils.arg('--name', nargs='?', metavar='', help='New name for backup.') @utils.arg('--description', metavar='', help='Backup description. Default=None.') -@utils.service_type('volumev3') -@api_versions.wraps('3.9') def do_backup_update(cs, args): """Renames a backup.""" kwargs = {}