Moved page argument validation to PageArgument and updated tests.

awscli/customizations/paginate.py

@@ -24,6 +24,7 @@
 
 """
 import logging
+import sys
 from functools import partial
 
 from botocore import xform_name
@@ -32,7 +33,7 @@
 
 from awscli.arguments import BaseCLIArgument
 from awscli.customizations.exceptions import ParamValidationError
-
+from awscli.customizations.utils import uni_print
 
 logger = logging.getLogger(__name__)
 
@@ -165,19 +166,16 @@ def unify_paging_params(argument_table, operation_model, event_name,
                                      serialized_name='MaxItems'),
                         shadowed_args)
 
-    # We will register three pagination handlers.
+    # We will register two pagination handlers.
     #
     # The first is focused on analyzing the CLI arguments passed to see
     # if they contain explicit operation-specific pagination args.  If so,
     # they are doing manual pagination and we turn off the CLI pagination.
     #
-    # The second analyzes the CLI arguments to see if the user has specified
-    # a valid value for the max-items argument, and warns them if not.
-    #
-    # The third is called later in the event chain and analyzes the actual
+    # The second is called later in the event chain and analyzes the actual
     # calling parameters passed to the operation.
     #
-    # The reason we have to do the third is that someone could put
+    # The reason we have to do the second is that someone could put
     # operation-specific pagination args in the CLI input JSON file
     # directly and this bypasses all of the CLI args processing.
     session.register(
@@ -186,10 +184,6 @@ def unify_paging_params(argument_table, operation_model, event_name,
                 list(_get_all_cli_input_tokens(paginator_config)),
                 shadowed_args, argument_table))
 
-    session.register(
-        parsed_args_event,
-        check_and_warn_negative_max_items)
-
     session.register(
         call_parameters_event,
         partial(check_should_enable_pagination_call_parameters,
@@ -234,13 +228,6 @@ def check_should_enable_pagination(input_tokens, shadowed_args, argument_table,
             argument_table[key] = value
 
 
-def check_and_warn_negative_max_items(parsed_args, parsed_globals, **kwargs):
-    parsed_max_items = getattr(parsed_args, 'max_items', None)
-    if parsed_globals.paginate and parsed_max_items is not None:
-        if int(parsed_max_items) <= 0:
-            logger.critical("Non-positive values for --max-items are unsupported and may yield undefined behavior.")
-
-
 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()]
@@ -353,7 +340,15 @@ def add_to_parser(self, parser):
         parser.add_argument(self.cli_name, dest=self.py_name,
                             type=self.type_map[self._parse_type])
 
+    def _check_and_warn_negative_max_items(self, value):
+        if value <= 0:
+            uni_print(
+                "warning: Non-positive values for --max-items are unsupported and may yield undefined behavior.\n",
+                sys.stderr)
+
     def add_to_params(self, parameters, value):
+        if self._serialized_name == 'MaxItems' and value is not None:
+            self._check_and_warn_negative_max_items(value)
         if value is not None:
             pagination_config = parameters.get('PaginationConfig', {})
             pagination_config[self._serialized_name] = value

tests/unit/customizations/test_paginate.py

@@ -10,6 +10,10 @@
 # distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF
 # ANY KIND, either express or implied. See the License for the specific
 # language governing permissions and limitations under the License.
+import contextlib
+import io
+
+from awscli.customizations.paginate import PageArgument
 from awscli.testutils import mock, unittest
 
 from botocore.exceptions import DataNotFoundError
@@ -19,10 +23,6 @@
 from awscli.customizations import paginate
 from awscli.customizations.exceptions import ParamValidationError
 
-import logging
-
-logger = logging.getLogger(__name__)
-
 class TestPaginateBase(unittest.TestCase):
 
     def setUp(self):
@@ -358,36 +358,20 @@ class TestNonPositiveIntWarnings(TestPaginateBase):
 
     def setUp(self):
         super(TestNonPositiveIntWarnings, self).setUp()
-        self.parsed_args = mock.Mock()
-        self.parsed_globals = mock.Mock()
-
-        self.parsed_globals.paginate = True
-
-        self.parsed_args.max_items = None
+        self._stderr = io.StringIO()
+        self._page_arg = PageArgument('max-items', 'documentation', int, 'MaxItems')
 
     def test_positive_integer_does_not_raise_warning(self):
-        self.parsed_args.max_items = 10
-        with self.assertLogs(level='CRITICAL') as log:
-            paginate.check_and_warn_negative_max_items(self.parsed_args, self.parsed_globals)
-            logger.critical('Log message')
-
-            # Since we logged exactly once above, asserting the length of the log output equals 1
-            # confirms that check_and_warn_negative_max_items did not log to critical level.
-            self.assertEqual(len(log.output), 1)
-            self.assertEqual(len(log.records), 1)
+        with contextlib.redirect_stderr(self._stderr):
+            self._page_arg.add_to_params({}, 1)
+            self.assertNotIn('Non-positive values for --max-items are unsupported', self._stderr.getvalue())
 
     def test_zero_raises_warning(self):
-        self.parsed_args.max_items = 0
-        with self.assertLogs(level='CRITICAL') as log:
-            paginate.check_and_warn_negative_max_items(self.parsed_args, self.parsed_globals)
-            self.assertEqual(len(log.output), 1)
-            self.assertEqual(len(log.records), 1)
-            self.assertIn("Non-positive values for --max-items are unsupported", log.output[0])
+        with contextlib.redirect_stderr(self._stderr):
+            self._page_arg.add_to_params({}, 0)
+            self.assertIn('Non-positive values for --max-items are unsupported', self._stderr.getvalue())
 
     def test_negative_integer_raises_warning(self):
-        self.parsed_args.max_items = -1
-        with self.assertLogs(level='CRITICAL') as log:
-            paginate.check_and_warn_negative_max_items(self.parsed_args, self.parsed_globals)
-            self.assertEqual(len(log.output), 1)
-            self.assertEqual(len(log.records), 1)
-            self.assertIn("Non-positive values for --max-items are unsupported", log.output[0])
+        with contextlib.redirect_stderr(self._stderr):
+            self._page_arg.add_to_params({}, -1)
+            self.assertIn('Non-positive values for --max-items are unsupported', self._stderr.getvalue())