From 33b5adbc2242b8736d9c600ce69ea2a30d83ac4e Mon Sep 17 00:00:00 2001 From: James Saryerwinnie Date: Tue, 7 Oct 2014 14:05:57 -0700 Subject: [PATCH 1/3] Fix error message for unknown keys We now include the correct CLI argument name in the error message, before we were using the xformed py_name, which would give use things like --foo_bar instead of --foo-bar. Error messages for unknown keys now looks like: ``` Error parsing parameter '--policy-attributes': Unknown key 'name', valid choices are: AttributeName, AttributeValue ``` --- awscli/argprocess.py | 12 ++++++------ tests/unit/test_argprocess.py | 15 +++++++++++---- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/awscli/argprocess.py b/awscli/argprocess.py index 155a3b2b3ed9..d83c8cc24533 100644 --- a/awscli/argprocess.py +++ b/awscli/argprocess.py @@ -52,11 +52,11 @@ class ParamSyntaxError(Exception): class ParamUnknownKeyError(Exception): - def __init__(self, param, key, valid_keys): + def __init__(self, key, valid_keys): valid_keys = ', '.join(valid_keys) full_message = ( - "Unknown key '%s' for parameter %s, valid choices " - "are: %s" % (key, '--%s' % xform_name(param.name), valid_keys)) + "Unknown key '%s', valid choices " + "are: %s" % (key, valid_keys)) super(ParamUnknownKeyError, self).__init__(full_message) @@ -284,7 +284,7 @@ def __call__(self, cli_argument, value, **kwargs): docgen = ParamShorthandDocGen() example_usage = docgen.generate_shorthand_example(cli_argument) raise ParamError(cli_argument.cli_name, "should be: %s" % example_usage) - except ParamError as e: + except (ParamError, ParamUnknownKeyError) as e: # The shorthand parse methods don't have the cli_name, # so any ParamError won't have this value. To accomodate # this, ParamErrors are caught and reraised with the cli_name @@ -362,7 +362,7 @@ def _struct_scalar_list_parse(self, param, value): # This is a key/value pair. current_key = current[0].strip() if current_key not in args: - raise ParamUnknownKeyError(param, current_key, + raise ParamUnknownKeyError(current_key, args.keys()) current_value = unpack_scalar_cli_arg(args[current_key], current[1].strip()) @@ -440,7 +440,7 @@ def _key_value_parse(self, param, value): key = key.strip() value = value.strip() if valid_names and key not in valid_names: - raise ParamUnknownKeyError(param, key, valid_names) + raise ParamUnknownKeyError(key, valid_names) if valid_names: sub_param = valid_names[key] if sub_param is not None: diff --git a/tests/unit/test_argprocess.py b/tests/unit/test_argprocess.py index e3fa4e2105fb..e07a5a274119 100644 --- a/tests/unit/test_argprocess.py +++ b/tests/unit/test_argprocess.py @@ -289,8 +289,15 @@ def test_error_messages_for_structure_scalar(self): def test_mispelled_param_name(self): p = self.get_param_model( 'elasticbeanstalk.CreateConfigurationTemplate.SourceConfiguration') - error_msg = 'valid choices.*ApplicationName' - with self.assertRaisesRegexp(ParamUnknownKeyError, error_msg): + # We're checking three things. + # 1) The CLI parameter is in the error message + # 2) The parameter name that failed validation is in the error message + # 3) The correct parameter name is in the error message. + error_msg = ( + '--source-configuration.*' + 'ApplicationNames.*valid choices.*' + 'ApplicationName') + with self.assertRaisesRegexp(ParamError, error_msg): # Typo in 'ApplicationName' self.simplify(p, 'ApplicationNames=foo, TemplateName=bar') @@ -312,8 +319,8 @@ def test_improper_separator_for_filters_param(self): def test_unknown_key_for_filters_param(self): p = self.get_param_model('ec2.DescribeInstances.Filters') - with self.assertRaisesRegexp(ParamUnknownKeyError, - 'valid choices.*Name'): + with self.assertRaisesRegexp(ParamError, + '--filters.*Names.*valid choices.*Name'): self.simplify(p, ["Names=instance-id,Values=foo,bar"]) def test_csv_syntax_escaped(self): From 4d3569c8ba0f7417ef3451d6aec82eb58d6d64a8 Mon Sep 17 00:00:00 2001 From: James Saryerwinnie Date: Tue, 7 Oct 2014 14:17:51 -0700 Subject: [PATCH 2/3] Fix bug for py3 in shorthand error message --- awscli/argprocess.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/awscli/argprocess.py b/awscli/argprocess.py index d83c8cc24533..8ba80cb79486 100644 --- a/awscli/argprocess.py +++ b/awscli/argprocess.py @@ -289,7 +289,7 @@ def __call__(self, cli_argument, value, **kwargs): # so any ParamError won't have this value. To accomodate # this, ParamErrors are caught and reraised with the cli_name # injected. - raise ParamError(cli_argument.cli_name, e.message) + raise ParamError(cli_argument.cli_name, str(e)) return parsed def get_parse_method_for_param(self, cli_argument, value=None): From e8da4d62991d24217b54f1e3982ca1ed187c1d9b Mon Sep 17 00:00:00 2001 From: James Saryerwinnie Date: Tue, 7 Oct 2014 14:18:11 -0700 Subject: [PATCH 3/3] Add bug fix to the changelog --- CHANGELOG.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 48cf11068b99..2d17b5cde11d 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -8,6 +8,9 @@ Next Release (TBD) * bugfix:S3 Response Parsing: Fix regression for parsing S3 responses containing a status code of 200 with an error response body (`botocore issue 342 `__) +* bugfix:Shorthand Error Message: Ensure the error message for + shorthand parsing always contains the CLI argument name + (`issue 935 `__) 1.5.0