Sitelet https://github.com/duyanh/aws-cli/commit/cb5dfc4cb0eb9a4919a76db1881dc83ba8c3d17b
Skip to content

Commit cb5dfc4

Browse files
jameslsjamesls
committed
Merge branch 'multi-json-arg' into develop
* multi-json-arg: Show better error message for invalid JSON Cleanup unused imports in test module
2 parents bb8b570 + 2a95d9b commit cb5dfc4

4 files changed

Lines changed: 53 additions & 20 deletions

File tree

‎awscli/argprocess.py‎

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,11 @@
2626

2727
class ParamError(Exception):
2828
def __init__(self, param, message):
29-
full_message = ("Error parsing parameter %s, should be: %s" %
29+
full_message = ("Error parsing parameter %s: %s" %
3030
(param.cli_name, message))
3131
super(ParamError, self).__init__(full_message)
3232
self.param = param
33+
self.message = message
3334

3435

3536
class ParamSyntaxError(Exception):
@@ -129,7 +130,7 @@ def __call__(self, param, value, **kwargs):
129130
if doc_fn is None:
130131
raise e
131132
else:
132-
raise ParamError(param, doc_fn(param))
133+
raise ParamError(param, "should be: %s" % doc_fn(param))
133134
return parsed
134135

135136
def get_parse_method_for_param(self, param, value=None):
@@ -353,12 +354,13 @@ def unpack_cli_arg(parameter, value):
353354
def unpack_complex_cli_arg(parameter, value):
354355
if parameter.type == 'structure' or parameter.type == 'map':
355356
if value.lstrip()[0] == '{':
356-
d = json.loads(value, object_pairs_hook=OrderedDict)
357-
else:
358-
msg = 'The value for parameter "%s" must be JSON or path to file.' % (
359-
parameter.cli_name)
360-
raise ValueError(msg)
361-
return d
357+
try:
358+
return json.loads(value, object_pairs_hook=OrderedDict)
359+
except ValueError as e:
360+
raise ParamError(
361+
parameter, "Invalid JSON: %s\nJSON received: %s"
362+
% (e, value))
363+
raise ParamError(parameter, "Invalid JSON:\n%s" % value)
362364
elif parameter.type == 'list':
363365
if isinstance(value, six.string_types):
364366
if value.lstrip()[0] == '[':
@@ -367,7 +369,14 @@ def unpack_complex_cli_arg(parameter, value):
367369
single_value = value[0].strip()
368370
if single_value and single_value[0] == '[':
369371
return json.loads(value[0], object_pairs_hook=OrderedDict)
370-
return [unpack_cli_arg(parameter.members, v) for v in value]
372+
try:
373+
return [unpack_cli_arg(parameter.members, v) for v in value]
374+
except ParamError as e:
375+
# The list params don't have a name/cli_name attached to them
376+
# so they will have bad error messages. We're going to
377+
# attach the parent parmeter to this error message to provide
378+
# a more helpful error message.
379+
raise ParamError(parameter, e.message)
371380

372381

373382
def unpack_scalar_cli_arg(parameter, value):
@@ -381,7 +390,7 @@ def unpack_scalar_cli_arg(parameter, value):
381390
file_path = os.path.expanduser(file_path)
382391
if not os.path.isfile(file_path):
383392
msg = 'Blob values must be a path to a file.'
384-
raise ValueError(msg)
393+
raise ParamError(parameter, msg)
385394
return open(file_path, 'rb')
386395
elif parameter.type == 'boolean':
387396
if isinstance(value, str) and value.lower() == 'false':

‎awscli/clidriver.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,7 @@ def main(self, args=None):
202202
except Exception as e:
203203
LOG.debug("Exception caught in main()", exc_info=True)
204204
LOG.debug("Exiting with rc 255")
205+
sys.stderr.write("\n")
205206
sys.stderr.write("%s\n" % e)
206207
return 255
207208

‎tests/unit/ec2/test_get_password_data.py‎

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,6 @@
1313
# language governing permissions and limitations under the License.
1414
from tests.unit import BaseAWSCommandParamsTest
1515
import os
16-
import re
17-
18-
from six.moves import cStringIO
19-
import mock
2016

2117

2218
PASSWORD_DATA = ("GWDnuoj/7pbMQkg125E8oGMUVCI+r98sGbFFl8SX+dEYxMZzz+byYwwjvyg8i"
@@ -38,7 +34,6 @@ def setUp(self):
3834
'PasswordData': PASSWORD_DATA}
3935

4036
def test_no_priv_launch_key(self):
41-
captured = cStringIO()
4237
args = ' --instance-id i-12345678'
4338
cmdline = self.prefix + args
4439
result = {'InstanceId': 'i-12345678'}
@@ -51,15 +46,13 @@ def test_nonexistent_priv_launch_key(self):
5146
args = ' --instance-id i-12345678 --priv-launch-key foo.pem'
5247
cmdline = self.prefix + args
5348
result = {}
54-
captured = cStringIO()
5549
error_msg = self.assert_params_for_cmd(
5650
cmdline, result, expected_rc=255)[1]
57-
self.assertEqual(error_msg, ('priv-launch-key should be a path to '
58-
'the local SSH private key file used '
59-
'to launch the instance.\n'))
51+
self.assertIn('priv-launch-key should be a path to '
52+
'the local SSH private key file used '
53+
'to launch the instance.\n', error_msg)
6054

6155
def test_priv_launch_key(self):
62-
captured = cStringIO()
6356
key_path = os.path.join(os.path.dirname(__file__),
6457
'testcli.pem')
6558
args = ' --instance-id i-12345678 --priv-launch-key %s' % key_path

‎tests/unit/test_argprocess.py‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -335,5 +335,35 @@ def test_gen_list_structure_multiple_scalar_docs(self):
335335
self.assertEqual(doc_string, s)
336336

337337

338+
class TestUnpackJSONParams(BaseArgProcessTest):
339+
def setUp(self):
340+
super(TestUnpackJSONParams, self).setUp()
341+
self.simplify = ParamShorthand()
342+
343+
def test_json_with_spaces(self):
344+
p = self.get_param_object('ec2.RunInstances.BlockDeviceMappings')
345+
# If a user specifies the json with spaces, it will show up as
346+
# a multi element list. For example:
347+
# --block-device-mappings [{ "DeviceName":"/dev/sdf",
348+
# "VirtualName":"ephemeral0"}, {"DeviceName":"/dev/sdg",
349+
# "VirtualName":"ephemeral1" }]
350+
#
351+
# Will show up as:
352+
block_device_mapping = [
353+
'[{', 'DeviceName:/dev/sdf,', 'VirtualName:ephemeral0},',
354+
'{DeviceName:/dev/sdg,', 'VirtualName:ephemeral1', '}]']
355+
# The shell has removed the double quotes so this is invalid
356+
# JSON, but we should still raise a better exception.
357+
with self.assertRaises(ParamError) as e:
358+
unpack_cli_arg(p, block_device_mapping)
359+
# Parameter name should be in error message.
360+
self.assertIn('--block-device-mappings', str(e.exception))
361+
# The actual JSON itself should be in the error message.
362+
# Becaues this is a list, only the first element in the JSON
363+
# will show. This will at least let customers know what
364+
# we tried to parse.
365+
self.assertIn('[{', str(e.exception))
366+
367+
338368
if __name__ == '__main__':
339369
unittest.main()

0 commit comments

Comments
 (0)