From b65f1b866826b366cb431cd7febe3a24f12157e0 Mon Sep 17 00:00:00 2001 From: James Saryerwinnie Date: Thu, 26 Jun 2014 15:56:49 -0700 Subject: [PATCH 1/9] Don't validate --endpoint-url for help subcommand "aws cloudsearchdomain" has --endpoint-url validation because it's a required option for that service. However, we should *not* be validating this when a user is just trying to get help. The fix is to move the validation to the operation-args-parsed.* event, which is after all the help processing code. I've also added integration tests that actually run the help command for cloudsearchdomain to verify this behavior works. --- awscli/customizations/cloudsearchdomain.py | 8 ++++---- awscli/help.py | 2 +- tests/integration/test_cli.py | 17 +++++++++++++++++ .../customizations/test_cloudsearchdomain.py | 19 +++++++++++++++---- 4 files changed, 37 insertions(+), 9 deletions(-) diff --git a/awscli/customizations/cloudsearchdomain.py b/awscli/customizations/cloudsearchdomain.py index 533c6fe8a512..c21ce9d151c0 100644 --- a/awscli/customizations/cloudsearchdomain.py +++ b/awscli/customizations/cloudsearchdomain.py @@ -19,11 +19,11 @@ """ def register_cloudsearchdomain(cli): - cli.register('top-level-args-parsed', validate_endpoint_url) + cli.register('operation-args-parsed.cloudsearchdomain', + validate_endpoint_url) -def validate_endpoint_url(parsed_args, **kwargs): - if parsed_args.command == 'cloudsearchdomain' and \ - parsed_args.endpoint_url is None: +def validate_endpoint_url(parsed_globals, **kwargs): + if parsed_globals.endpoint_url is None: raise ValueError( "--endpoint-url is required for cloudsearchdomain commands") diff --git a/awscli/help.py b/awscli/help.py index 25218a427b3f..5a695ed0b74c 100644 --- a/awscli/help.py +++ b/awscli/help.py @@ -114,7 +114,7 @@ def _exists_on_path(self, name): # Since we're only dealing with POSIX systems, we can # ignore things like PATHEXT. return any([os.path.exists(os.path.join(p, name)) - for p in os.environ.get('PATH', []).split(os.pathsep)]) + for p in os.environ.get('PATH', '').split(os.pathsep)]) def _popen(self, *args, **kwargs): return Popen(*args, **kwargs) diff --git a/tests/integration/test_cli.py b/tests/integration/test_cli.py index d96a524386f3..edbac3fa9fc2 100644 --- a/tests/integration/test_cli.py +++ b/tests/integration/test_cli.py @@ -76,6 +76,23 @@ def test_operation_help_with_required_arg(self): self.assertEqual(p.rc, 1, p.stderr) self.assertIn('get-object', p.stdout) + def test_service_help_with_required_option(self): + # In cloudsearchdomain, the --endpoint-url is required. + # We want to make sure if you're just getting help tex + # that we don't trigger that validation. + p = aws('cloudsearchdomain help') + self.assertEqual(p.rc, 1, p.stderr) + self.assertIn('cloudsearchdomain', p.stdout) + # And nothing on stderr about missing options. + self.assertEqual(p.stderr, '') + + def test_operation_help_with_required_option(self): + p = aws('cloudsearchdomain search help') + self.assertEqual(p.rc, 1, p.stderr) + self.assertIn('search', p.stdout) + # And nothing on stderr about missing options. + self.assertEqual(p.stderr, '') + def test_help_with_warning_blocks(self): p = aws('elastictranscoder create-pipeline help') self.assertEqual(p.rc, 1, p.stderr) diff --git a/tests/unit/customizations/test_cloudsearchdomain.py b/tests/unit/customizations/test_cloudsearchdomain.py index 8ba25137c980..211ccd404e2a 100644 --- a/tests/unit/customizations/test_cloudsearchdomain.py +++ b/tests/unit/customizations/test_cloudsearchdomain.py @@ -12,6 +12,7 @@ # language governing permissions and limitations under the License. from awscli.testutils import unittest from awscli.testutils import BaseAWSCommandParamsTest +from awscli.help import HelpRenderer from awscli.customizations.cloudsearchdomain import validate_endpoint_url import mock @@ -48,14 +49,24 @@ def test_endpoint_is_required(self): stderr = self.run_cmd(cmd, expected_rc=255)[1] self.assertIn('--endpoint-url is required', stderr) + def test_endpoint_not_required_for_help(self): + cmd = self.prefix + 'help' + with mock.patch('awscli.help.get_renderer') as get_renderer: + mock_render = mock.Mock(spec=HelpRenderer) + get_renderer.return_value = mock_render + stdout, stderr, rc = self.run_cmd(cmd, expected_rc=None) + # If we get this far we've succeeded, but we can do + # a quick sanity check and make sure the service name is + # in the stdout help text. + self.assertIn(stdout, 'cloudsearchdomain') + class TestCloudsearchDomainHandler(unittest.TestCase): def test_validate_endpoint_url_is_none(self): - parsed_args = mock.Mock() - parsed_args.endpoint_url = None - parsed_args.command = 'cloudsearchdomain' + parsed_globals = mock.Mock() + parsed_globals.endpoint_url = None with self.assertRaises(ValueError): - validate_endpoint_url(parsed_args) + validate_endpoint_url(parsed_globals) if __name__ == "__main__": From c2580e0bd3e780c1a7dcbc118d6ce6c48904590c Mon Sep 17 00:00:00 2001 From: James Saryerwinnie Date: Thu, 26 Jun 2014 16:08:20 -0700 Subject: [PATCH 2/9] Update changelog with issue 828 --- CHANGELOG.rst | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index a158311a24c5..94eb88d06762 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -2,6 +2,14 @@ CHANGELOG ========= +Next Release (TBD) +================== + +* bugfix:``aws cloudsearchdomain``: Fix bug where + ``--endpoint-url`` is required even for ``help`` subcommands + (`issue 828 `__) + + 1.3.20 ====== From 9bb39c0310e50d60e81a1568f61a7de34444bd4c Mon Sep 17 00:00:00 2001 From: Tim G Date: Fri, 27 Jun 2014 14:59:58 +1000 Subject: [PATCH 3/9] bugfix: convert ints to strings before write Ensures all scalar values (like ints) are converted to string types before write. --- awscli/text.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/awscli/text.py b/awscli/text.py index cb370d10ac3a..d74d4cb0dd1d 100644 --- a/awscli/text.py +++ b/awscli/text.py @@ -25,7 +25,7 @@ def _format_text(item, stream, identifier=None, scalar_keys=None): else: # If it's not a list or a dict, we just write the scalar # value out directly. - stream.write(item) + stream.write(six.text_type(item)) stream.write('\n') From be0dd805dc38f8c5feb694d0b69d3f0c576f93e2 Mon Sep 17 00:00:00 2001 From: "Daniel G. Taylor" Date: Mon, 30 Jun 2014 13:44:05 -0700 Subject: [PATCH 4/9] Fix single item shorthand list parsing bug This fixes a bug where shorthand lists made with `[` and `]` containing only a single item would not be parsed correctly. The `_eat_items` utility function did not take into account the fact that the list end character could be in the first item. I've added a conditional to check for this case and not call the `_eat_items` method, because the current behavior is expected for some inputs that use `_eat_items`. --- awscli/utils.py | 6 +++++- tests/unit/test_argprocess.py | 10 ++++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/awscli/utils.py b/awscli/utils.py index 414474f1bc29..bd37243fcbf5 100644 --- a/awscli/utils.py +++ b/awscli/utils.py @@ -45,7 +45,11 @@ def _split_with_quotes(value): if list_start >= 0 and value.find(']') != -1 and \ (quote_char is None or part.find(quote_char) > list_start): # This is a list, eat all the items until the end - new_chunk = _eat_items(value, iter_parts, part, ']') + if ']' in part: + # Short circuit for only one item + new_chunk = part + else: + new_chunk = _eat_items(value, iter_parts, part, ']') list_items = _split_with_quotes(new_chunk[list_start + 2:-1]) new_chunk = new_chunk[:list_start + 1] + ','.join(list_items) new_parts.append(new_chunk) diff --git a/tests/unit/test_argprocess.py b/tests/unit/test_argprocess.py index 3356006cc659..207ccd8e3f50 100644 --- a/tests/unit/test_argprocess.py +++ b/tests/unit/test_argprocess.py @@ -220,12 +220,18 @@ def test_list_structure_list_scalar_3(self): {"Name": "foo", "Args": ["a", "k1=v1", "b"]}, {"Name": "bar", - "Args": ["baz"]} + "Args": ["baz"]}, + {"Name": "single_kv", + "Args": ["key=value"]}, + {"Name": "single_v", + "Args": ["value"]} ] simplified = self.simplify(p, [ "Name=foo,Args=[a,k1=v1,b]", - "Name=bar,Args=baz" + "Name=bar,Args=baz", + "Name=single_kv,Args=[key=value]", + "Name=single_v,Args=[value]" ]) self.assertEqual(simplified, expected) From db9538bdff55646e75d8c72bd1947fffa5078f33 Mon Sep 17 00:00:00 2001 From: "Daniel G. Taylor" Date: Mon, 30 Jun 2014 14:24:14 -0700 Subject: [PATCH 5/9] Update changelog --- CHANGELOG.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 94eb88d06762..e7d1f0af7de3 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -5,6 +5,10 @@ CHANGELOG Next Release (TBD) ================== +* bugfix:Shorthand JSON: Fix bug where shorthand lists with + a single item (e.g. ``--arg Param=[item]``) were not parsed + correctly. + (`issue 830 `__) * bugfix:``aws cloudsearchdomain``: Fix bug where ``--endpoint-url`` is required even for ``help`` subcommands (`issue 828 `__) From d421dc264819c8668cac775a291744afafdcd77b Mon Sep 17 00:00:00 2001 From: James Saryerwinnie Date: Mon, 30 Jun 2014 15:18:07 -0700 Subject: [PATCH 6/9] Update changelog with opsworks update --- CHANGELOG.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index e7d1f0af7de3..a4f08673bd3d 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -5,6 +5,8 @@ CHANGELOG Next Release (TBD) ================== +* feature:``aws opsworks``: Update the ``aws opsworks`` command + to the latest version * bugfix:Shorthand JSON: Fix bug where shorthand lists with a single item (e.g. ``--arg Param=[item]``) were not parsed correctly. From 4ac8c778731e668323251cc6b32c7ea695e41ad6 Mon Sep 17 00:00:00 2001 From: James Saryerwinnie Date: Mon, 30 Jun 2014 16:52:22 -0700 Subject: [PATCH 7/9] Add unittests for #829 --- tests/unit/test_text.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/unit/test_text.py b/tests/unit/test_text.py index 4c28ef4c33f9..341053efe03d 100644 --- a/tests/unit/test_text.py +++ b/tests/unit/test_text.py @@ -61,6 +61,15 @@ def test_multiple_list_of_dicts(self): 'ZOO\t0\t1\t2\n' ) + def test_single_scalar_number(self): + self.assert_text_renders_to(10, '10\n') + + def test_list_of_single_number(self): + self.assert_text_renders_to([10], '10\n') + + def test_list_of_multiple_numbers(self): + self.assert_text_renders_to([10, 10, 10], '10\t10\t10\n') + def test_different_keys_in_sublists(self): self.assert_text_renders_to( # missing "b" adds "d" From 84ee524ddbb56ba3242b2f59444667c842826ba0 Mon Sep 17 00:00:00 2001 From: James Saryerwinnie Date: Mon, 30 Jun 2014 16:53:37 -0700 Subject: [PATCH 8/9] Add changelog entry for #829 --- CHANGELOG.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 94eb88d06762..f0ddb3626641 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -5,6 +5,9 @@ CHANGELOG Next Release (TBD) ================== +* bugfix:Text output: Fix bug when rendering only + scalars that are numbers in text output + (`issue 829 `__) * bugfix:``aws cloudsearchdomain``: Fix bug where ``--endpoint-url`` is required even for ``help`` subcommands (`issue 828 `__) From 69fbea5462c6cea71a847bad70891e4ccd452990 Mon Sep 17 00:00:00 2001 From: James Saryerwinnie Date: Tue, 1 Jul 2014 14:38:53 -0700 Subject: [PATCH 9/9] Bumping version to 1.3.21 --- CHANGELOG.rst | 4 ++-- awscli/__init__.py | 2 +- doc/source/conf.py | 2 +- setup.py | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 84d6298c8421..dad42f72b3f9 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -2,8 +2,8 @@ CHANGELOG ========= -Next Release (TBD) -================== +1.3.21 +====== * feature:``aws opsworks``: Update the ``aws opsworks`` command to the latest version diff --git a/awscli/__init__.py b/awscli/__init__.py index bcc14bf96417..6aea8984ed37 100644 --- a/awscli/__init__.py +++ b/awscli/__init__.py @@ -17,7 +17,7 @@ """ import os -__version__ = '1.3.20' +__version__ = '1.3.21' # # Get our data path to be added to botocore's search path diff --git a/doc/source/conf.py b/doc/source/conf.py index 648996a20b2d..426a233c4ed6 100644 --- a/doc/source/conf.py +++ b/doc/source/conf.py @@ -52,7 +52,7 @@ # The short X.Y version. version = '1.3.' # The full version, including alpha/beta/rc tags. -release = '1.3.20' +release = '1.3.21' # The language for content autogenerated by Sphinx. Refer to documentation # for a list of supported languages. diff --git a/setup.py b/setup.py index ec1e88c7cd1d..f3ba063dc590 100644 --- a/setup.py +++ b/setup.py @@ -6,7 +6,7 @@ import awscli -requires = ['botocore>=0.54.0,<0.55.0', +requires = ['botocore>=0.55.0,<0.56.0', 'bcdoc>=0.12.0,<0.13.0', 'six>=1.1.0', 'colorama==0.2.5',