]> review.fuel-infra Code Review - openstack-build/cinder-build.git/commitdiff
Parameter osapi_max_limit is always used by default
authorYuriy Nesenenko <ynesenenko@mirantis.com>
Tue, 25 Aug 2015 10:55:56 +0000 (13:55 +0300)
committerYuriy Nesenenko <ynesenenko@mirantis.com>
Tue, 25 Aug 2015 19:52:20 +0000 (19:52 +0000)
Default value max_limit is initialized before a value is read from
the config file. Default parameter values are always evaluated when
the “def” statement they belong to is executed. So parameter
osapi_max_limit is always used by default even it's set in the
configuration file.

Change-Id: I1414667d30e98b56c5eb1b09b802bab823b37e2d
Closes-Bug: #1487510

cinder/api/common.py
cinder/tests/unit/api/test_common.py

index 06fee0008cc26c911a9a209ea66885bed910b005..9dc7ed11215b60417fe1840fb304954df7d8cb19 100644 (file)
@@ -69,7 +69,7 @@ def validate_key_names(key_names_list):
     return True
 
 
-def get_pagination_params(params, max_limit=CONF.osapi_max_limit):
+def get_pagination_params(params, max_limit=None):
     """Return marker, limit, offset tuple from request.
 
     :param params: `wsgi.Request`'s GET dictionary, possibly containing
@@ -85,18 +85,20 @@ def get_pagination_params(params, max_limit=CONF.osapi_max_limit):
     :max_limit: Max value 'limit' return value can take
     :returns: Tuple (marker, limit, offset)
     """
+    max_limit = max_limit or CONF.osapi_max_limit
     limit = _get_limit_param(params, max_limit)
     marker = _get_marker_param(params)
     offset = _get_offset_param(params)
     return marker, limit, offset
 
 
-def _get_limit_param(params, max_limit=CONF.osapi_max_limit):
+def _get_limit_param(params, max_limit=None):
     """Extract integer limit from request's dictionary or fail.
 
    Defaults to max_limit if not present and returns max_limit if present
    'limit' is greater than max_limit.
     """
+    max_limit = max_limit or CONF.osapi_max_limit
     try:
         limit = int(params.pop('limit', max_limit))
     except ValueError:
@@ -129,7 +131,7 @@ def _get_offset_param(params):
     return offset
 
 
-def limited(items, request, max_limit=CONF.osapi_max_limit):
+def limited(items, request, max_limit=None):
     """Return a slice of items according to requested offset and limit.
 
     :param items: A sliceable entity
@@ -141,14 +143,16 @@ def limited(items, request, max_limit=CONF.osapi_max_limit):
                     will cause exc.HTTPBadRequest() exceptions to be raised.
     :kwarg max_limit: The maximum number of items to return from 'items'
     """
+    max_limit = max_limit or CONF.osapi_max_limit
     marker, limit, offset = get_pagination_params(request.GET.copy(),
                                                   max_limit)
     range_end = offset + (limit or max_limit)
     return items[offset:range_end]
 
 
-def limited_by_marker(items, request, max_limit=CONF.osapi_max_limit):
+def limited_by_marker(items, request, max_limit=None):
     """Return a slice of items according to the requested marker and limit."""
+    max_limit = max_limit or CONF.osapi_max_limit
     marker, limit, __ = get_pagination_params(request.GET.copy(), max_limit)
 
     start_index = 0
index 8050a5e7d330951752df424300710f973ff3652e..ca7cbf1a75146da39264d4e926dd0caa910d200e 100644 (file)
@@ -178,10 +178,12 @@ class PaginationParamsTest(test.TestCase):
             webob.exc.HTTPBadRequest, common.get_pagination_params,
             req.GET.copy())
 
-    def test_no_params(self):
+    @mock.patch.object(common, 'CONF')
+    def test_no_params(self, mock_cfg):
         """Test no params."""
+        mock_cfg.osapi_max_limit = 100
         req = webob.Request.blank('/')
-        expected = (None, CONF.osapi_max_limit, 0)
+        expected = (None, 100, 0)
         self.assertEqual(expected,
                          common.get_pagination_params(req.GET.copy()))