diff --git a/.changes/next-release/bugfix-Paginator.json b/.changes/next-release/bugfix-Paginator.json new file mode 100644 index 000000000000..a69ff23534c3 --- /dev/null +++ b/.changes/next-release/bugfix-Paginator.json @@ -0,0 +1,5 @@ +{ + "description": "Print a better error when pagination params are supplied along with no-paginate.", + "category": "Paginator", + "type": "bugfix" +} diff --git a/awscli/clidriver.py b/awscli/clidriver.py index 5dc0d836b294..df8594e0c975 100644 --- a/awscli/clidriver.py +++ b/awscli/clidriver.py @@ -516,8 +516,9 @@ def __call__(self, args, parsed_globals): self._name) self._emit(event, parsed_args=parsed_args, parsed_globals=parsed_globals) - call_parameters = self._build_call_parameters(parsed_args, - self.arg_table) + call_parameters = self._build_call_parameters( + parsed_args, self.arg_table) + event = 'calling-command.%s.%s' % (self._parent_name, self._name) override = self._emit_first_non_none_response( diff --git a/awscli/customizations/paginate.py b/awscli/customizations/paginate.py index 79258f666eed..21d871022d77 100644 --- a/awscli/customizations/paginate.py +++ b/awscli/customizations/paginate.py @@ -27,7 +27,7 @@ from functools import partial from botocore import xform_name -from botocore.exceptions import DataNotFoundError +from botocore.exceptions import DataNotFoundError, PaginationError from botocore import model from awscli.arguments import BaseCLIArgument @@ -173,7 +173,8 @@ def check_should_enable_pagination(input_tokens, shadowed_args, argument_table, logger.debug("User has specified a manual pagination arg. " "Automatically setting --no-paginate.") parsed_globals.paginate = False - # Because we've now disabled pagination, there's a chance that + + # Because pagination is now disabled, there's a chance that # we were shadowing arguments. For example, we inject a # --max-items argument in unify_paging_params(). If the # the operation also provides its own MaxItems (which we @@ -183,6 +184,23 @@ def check_should_enable_pagination(input_tokens, shadowed_args, argument_table, # what we're doing here. for key, value in shadowed_args.items(): argument_table[key] = value + + if not parsed_globals.paginate: + ensure_paging_params_not_set(parsed_args, shadowed_args) + + +def ensure_paging_params_not_set(parsed_args, shadowed_args): + paging_params = ['starting_token', 'page_size', 'max_items'] + shadowed_params = [p.replace('-', '_') for p in shadowed_args.keys()] + params_used = [p for p in paging_params if + p not in shadowed_params and getattr(parsed_args, p)] + + if len(params_used) > 0: + converted_params = ', '.join( + ["--" + p.replace('_', '-') for p in params_used]) + raise PaginationError( + message="Cannot specify --no-paginate along with pagination " + "arguments: %s" % converted_params) def _remove_existing_paging_arguments(argument_table, pagination_config): diff --git a/tests/functional/s3api/test_list_objects.py b/tests/functional/s3api/test_list_objects.py index 4e00bd21a9f0..70dd29ea372f 100644 --- a/tests/functional/s3api/test_list_objects.py +++ b/tests/functional/s3api/test_list_objects.py @@ -81,3 +81,12 @@ def test_max_keys_can_be_specified(self): self.assertEqual(len(self.operations_called), 1) self.assertEqual(len(self.operations_called), 1) self.assertEqual(self.operations_called[0][0].name, 'ListObjects') + + def test_pagination_params_cannot_be_supplied_with_no_paginate(self): + cmdline = self.prefix + ' --bucket mybucket --no-paginate ' \ + '--max-items 100' + self.assert_params_for_cmd( + cmdline, expected_rc=255, + stderr_contains="Error during pagination: Cannot specify " + "--no-paginate along with pagination arguments: " + "--max-items") diff --git a/tests/unit/customizations/test_paginate.py b/tests/unit/customizations/test_paginate.py index 70bbcb535fbe..1a1d0d7d5cdb 100644 --- a/tests/unit/customizations/test_paginate.py +++ b/tests/unit/customizations/test_paginate.py @@ -12,7 +12,7 @@ # language governing permissions and limitations under the License. from awscli.testutils import unittest -from botocore.exceptions import DataNotFoundError +from botocore.exceptions import DataNotFoundError, PaginationError from botocore.model import OperationModel from awscli.help import OperationHelpCommand, OperationDocumentEventHandler @@ -209,6 +209,9 @@ def setUp(self): super(TestShouldEnablePagination, self).setUp() self.parsed_globals = mock.Mock() self.parsed_args = mock.Mock() + self.parsed_args.starting_token = None + self.parsed_args.page_size = None + self.parsed_args.max_items = None def test_should_not_enable_pagination(self): # Here the user has specified a manual pagination argument, @@ -253,7 +256,7 @@ def test_default_to_pagination_on_when_ambiguous(self): self.assertTrue(self.parsed_globals.paginate, "Pagination was not enabled.") - def test_shadowed_args_are_replaced_when_pagination_off(self): + def test_shadowed_args_are_replaced_when_pagination_turned_off(self): input_tokens = ['foo', 'bar'] self.parsed_globals.paginate = True # Corresponds to --bar 10 @@ -268,3 +271,35 @@ def test_shadowed_args_are_replaced_when_pagination_off(self): # user specified --bar 10 self.assertFalse(self.parsed_globals.paginate) self.assertEqual(arg_table['foo'], mock.sentinel.ORIGINAL_ARG) + + def test_shadowed_args_are_replaced_when_pagination_set_off(self): + input_tokens = ['foo', 'bar'] + self.parsed_globals.paginate = False + # Corresponds to --bar 10 + self.parsed_args.foo = None + self.parsed_args.bar = 10 + shadowed_args = {'foo': mock.sentinel.ORIGINAL_ARG} + arg_table = {'foo': mock.sentinel.PAGINATION_ARG} + paginate.check_should_enable_pagination( + input_tokens, shadowed_args, arg_table, + self.parsed_args, self.parsed_globals) + # We should have turned paginate off because the + # user specified --bar 10 + self.assertFalse(self.parsed_globals.paginate) + self.assertEqual(arg_table['foo'], mock.sentinel.ORIGINAL_ARG) + + +class TestEnsurePagingParamsNotSet(TestPaginateBase): + def setUp(self): + super(TestEnsurePagingParamsNotSet, self).setUp() + self.parsed_args = mock.Mock() + + self.parsed_args.starting_token = None + self.parsed_args.page_size = None + self.parsed_args.max_items = None + + def test_pagination_params_raise_error_with_no_paginate(self): + self.parsed_args.max_items = 100 + + with self.assertRaises(PaginationError): + paginate.ensure_paging_params_not_set(self.parsed_args, {})