Merge pull request #1958 from JordonPhillips/really-no-paginate

Print better error when using pagination params and no paginate

.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"
+}

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(

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):

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")

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, {})