From 2443277edca6700df3d6714d5765d2f6547c01c3 Mon Sep 17 00:00:00 2001 From: "Daniel G. Taylor" Date: Tue, 4 Mar 2014 14:00:32 -0800 Subject: [PATCH 1/4] Fix pagination with string params --- awscli/customizations/paginate.py | 13 ++++++++++++- tests/unit/customizations/test_paginate.py | 4 ++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/awscli/customizations/paginate.py b/awscli/customizations/paginate.py index 51620eed8876..7ca6cc788cd9 100644 --- a/awscli/customizations/paginate.py +++ b/awscli/customizations/paginate.py @@ -66,8 +66,19 @@ def unify_paging_params(argument_table, operation, event_name, **kwargs): STARTING_TOKEN_HELP, operation, parse_type='string') + # Try to get the pagination parameter type + type_ = 'integer' + if 'limit_key' in operation.pagination: + for param in operation.params: + if param.name == operation.pagination['limit_key']: + type_ = param.type + break + + if type_ not in PageArgument.type_map: + raise TypeError('Unsupported pagination type {0}'.format(type_)) + argument_table['max-items'] = PageArgument('max-items', MAX_ITEMS_HELP, - operation, parse_type='integer') + operation, parse_type=type_) def check_should_enable_pagination(input_tokens, parsed_args, parsed_globals, **kwargs): diff --git a/tests/unit/customizations/test_paginate.py b/tests/unit/customizations/test_paginate.py index 298a99608730..2ccb455056cc 100644 --- a/tests/unit/customizations/test_paginate.py +++ b/tests/unit/customizations/test_paginate.py @@ -25,8 +25,10 @@ def setUp(self): self.foo_param = mock.Mock() self.foo_param.cli_name = 'foo' self.foo_param.name = 'Foo' + self.foo_param.type = 'string' self.bar_param = mock.Mock() self.bar_param.cli_name = 'bar' + self.bar_param.type = 'string' self.bar_param.name = 'Bar' self.params = [self.foo_param, self.bar_param] self.operation.pagination = { @@ -54,6 +56,8 @@ def test_customize_arg_table(self): paginate.PageArgument) self.assertIsInstance(argument_table['max-items'], paginate.PageArgument) + # Max items should be the same type as bar, which may not be an int + self.assertEqual('string', argument_table['max-items']._parse_type) def test_operation_with_no_paginate(self): # Operations that don't paginate are left alone. From 0ee286e76683f7923f32476424f9064ff540359c Mon Sep 17 00:00:00 2001 From: "Daniel G. Taylor" Date: Tue, 4 Mar 2014 15:51:52 -0800 Subject: [PATCH 2/4] More useful error and added tests --- awscli/customizations/paginate.py | 11 +++-- tests/unit/customizations/test_paginate.py | 57 ++++++++++++++++++++-- 2 files changed, 61 insertions(+), 7 deletions(-) diff --git a/awscli/customizations/paginate.py b/awscli/customizations/paginate.py index 7ca6cc788cd9..ac94bb46d63d 100644 --- a/awscli/customizations/paginate.py +++ b/awscli/customizations/paginate.py @@ -67,15 +67,18 @@ def unify_paging_params(argument_table, operation, event_name, **kwargs): operation, parse_type='string') # Try to get the pagination parameter type - type_ = 'integer' + limit_param = None if 'limit_key' in operation.pagination: for param in operation.params: if param.name == operation.pagination['limit_key']: - type_ = param.type + limit_param = param break - if type_ not in PageArgument.type_map: - raise TypeError('Unsupported pagination type {0}'.format(type_)) + type_ = limit_param and limit_param.type or 'integer' + if limit_param and limit_param.type not in PageArgument.type_map: + raise TypeError(('Unsupported pagination type {0} for operation {1}' + ' and parameter {2}').format(type_, operation.name, + limit_param.name)) argument_table['max-items'] = PageArgument('max-items', MAX_ITEMS_HELP, operation, parse_type=type_) diff --git a/tests/unit/customizations/test_paginate.py b/tests/unit/customizations/test_paginate.py index 2ccb455056cc..8724b22792ef 100644 --- a/tests/unit/customizations/test_paginate.py +++ b/tests/unit/customizations/test_paginate.py @@ -17,7 +17,7 @@ from awscli.customizations import paginate -class TestArgumentTableModifications(unittest.TestCase): +class TestPaginateBase(unittest.TestCase): def setUp(self): self.operation = mock.Mock() @@ -37,6 +37,9 @@ def setUp(self): } self.operation.params = self.params + +class TestArgumentTableModifications(TestPaginateBase): + def test_customize_arg_table(self): argument_table = { 'foo': mock.Mock(), @@ -56,8 +59,6 @@ def test_customize_arg_table(self): paginate.PageArgument) self.assertIsInstance(argument_table['max-items'], paginate.PageArgument) - # Max items should be the same type as bar, which may not be an int - self.assertEqual('string', argument_table['max-items']._parse_type) def test_operation_with_no_paginate(self): # Operations that don't paginate are left alone. @@ -70,3 +71,53 @@ def test_operation_with_no_paginate(self): paginate.unify_paging_params(argument_table, self.operation, 'building-argument-table.foo.bar') self.assertEqual(starting_table, argument_table) + + +class TestStringLimitKey(TestPaginateBase): + + def setUp(self): + super(TestStringLimitKey, self).setUp() + self.bar_param.type = 'string' + + def test_integer_limit_key(self): + argument_table = { + 'foo': mock.Mock(), + 'bar': mock.Mock(), + } + paginate.unify_paging_params(argument_table, self.operation, + 'building-argument-table.foo.bar') + # Max items should be the same type as bar, which may not be an int + self.assertEqual('string', argument_table['max-items']._parse_type) + + +class TestIntegerLimitKey(TestPaginateBase): + + def setUp(self): + super(TestIntegerLimitKey, self).setUp() + self.bar_param.type = 'integer' + + def test_integer_limit_key(self): + argument_table = { + 'foo': mock.Mock(), + 'bar': mock.Mock(), + } + paginate.unify_paging_params(argument_table, self.operation, + 'building-argument-table.foo.bar') + # Max items should be the same type as bar, which may not be an int + self.assertEqual('integer', argument_table['max-items']._parse_type) + + +class TestBadLimitKey(TestPaginateBase): + + def setUp(self): + super(TestBadLimitKey, self).setUp() + self.bar_param.type = 'bad' + + def test_integer_limit_key(self): + argument_table = { + 'foo': mock.Mock(), + 'bar': mock.Mock(), + } + with self.assertRaises(TypeError): + paginate.unify_paging_params(argument_table, self.operation, + 'building-argument-table.foo.bar') From bc02513f9a68a16fa758f9e755f6cfa35ae69440 Mon Sep 17 00:00:00 2001 From: "Daniel G. Taylor" Date: Tue, 4 Mar 2014 16:00:10 -0800 Subject: [PATCH 3/4] Add Route53 max items test case --- tests/unit/route53/test_resource_id.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/tests/unit/route53/test_resource_id.py b/tests/unit/route53/test_resource_id.py index 6f8a2f20fa55..9c58c670c072 100644 --- a/tests/unit/route53/test_resource_id.py +++ b/tests/unit/route53/test_resource_id.py @@ -102,4 +102,20 @@ def test_short_resource_id(self): self.assert_params_for_cmd(cmdline, result, expected_rc=0, ignore_params=['payload'])[0] - + +class TestMaxItems(BaseAWSCommandParamsTest): + + prefix = 'route53 list-resource-record-sets' + + def test_full_resource_id(self): + args = ' --hosted-zone-id /hostedzone/ABCD --max-items 1' + cmdline = self.prefix + args + result = { + 'uri_params': { + 'HostedZoneId': 'ABCD', + 'MaxItems': '1' + }, + 'headers': {} + } + self.assert_params_for_cmd(cmdline, result, expected_rc=0, + ignore_params=['payload'])[0] From 05044ccad4beb2694bb101dabfb17bd7a4f9e96d Mon Sep 17 00:00:00 2001 From: "Daniel G. Taylor" Date: Wed, 5 Mar 2014 11:27:34 -0800 Subject: [PATCH 4/4] Use public rather than private members --- tests/unit/customizations/test_paginate.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/customizations/test_paginate.py b/tests/unit/customizations/test_paginate.py index 8724b22792ef..05ef10053fd2 100644 --- a/tests/unit/customizations/test_paginate.py +++ b/tests/unit/customizations/test_paginate.py @@ -87,7 +87,7 @@ def test_integer_limit_key(self): paginate.unify_paging_params(argument_table, self.operation, 'building-argument-table.foo.bar') # Max items should be the same type as bar, which may not be an int - self.assertEqual('string', argument_table['max-items']._parse_type) + self.assertEqual('string', argument_table['max-items'].cli_type_name) class TestIntegerLimitKey(TestPaginateBase): @@ -104,7 +104,7 @@ def test_integer_limit_key(self): paginate.unify_paging_params(argument_table, self.operation, 'building-argument-table.foo.bar') # Max items should be the same type as bar, which may not be an int - self.assertEqual('integer', argument_table['max-items']._parse_type) + self.assertEqual('integer', argument_table['max-items'].cli_type_name) class TestBadLimitKey(TestPaginateBase):