Fixes Bug #803563 by changing how nova passes options in to glance. Before, if limit or marker were not set, we would pass limit=0 and marker=0 in to glance. However, marker is supposed to be an image id. With this change, if limit or marker are not set, they are simply not passed into glance. Glance is free then to choose the default behavior.

This commit is contained in:
Mark Washenberger 2011-06-29 19:17:00 +00:00 committed by Tarmac
commit 85522bba82
4 changed files with 41 additions and 38 deletions

View File

@ -45,23 +45,20 @@ def get_pagination_params(request):
exc.HTTPBadRequest() exceptions to be raised.
"""
try:
marker = int(request.GET.get('marker', 0))
except ValueError:
raise webob.exc.HTTPBadRequest(_('marker param must be an integer'))
params = {}
for param in ['marker', 'limit']:
if not param in request.GET:
continue
try:
params[param] = int(request.GET[param])
except ValueError:
msg = _('%s param must be an integer') % param
raise webob.exc.HTTPBadRequest(msg)
if params[param] < 0:
msg = _('%s param must be positive') % param
raise webob.exc.HTTPBadRequest(msg)
try:
limit = int(request.GET.get('limit', 0))
except ValueError:
raise webob.exc.HTTPBadRequest(_('limit param must be an integer'))
if limit < 0:
raise webob.exc.HTTPBadRequest(_('limit param must be positive'))
if marker < 0:
raise webob.exc.HTTPBadRequest(_('marker param must be positive'))
return(marker, limit)
return params
def limited(items, request, max_limit=FLAGS.osapi_max_limit):
@ -100,10 +97,10 @@ def limited(items, request, max_limit=FLAGS.osapi_max_limit):
def limited_by_marker(items, request, max_limit=FLAGS.osapi_max_limit):
"""Return a slice of items according to the requested marker and limit."""
(marker, limit) = get_pagination_params(request)
params = get_pagination_params(request)
if limit == 0:
limit = max_limit
limit = params.get('limit', max_limit)
marker = params.get('marker')
limit = min(max_limit, limit)
start_index = 0

View File

@ -181,9 +181,9 @@ class ControllerV11(Controller):
"""
context = req.environ['nova.context']
filters = self._get_filters(req)
(marker, limit) = common.get_pagination_params(req)
images = self._image_service.index(
context, filters=filters, marker=marker, limit=limit)
page_params = common.get_pagination_params(req)
images = self._image_service.index(context, filters=filters,
**page_params)
builder = self.get_builder(req).build
return dict(images=[builder(image, detail=False) for image in images])
@ -195,9 +195,9 @@ class ControllerV11(Controller):
"""
context = req.environ['nova.context']
filters = self._get_filters(req)
(marker, limit) = common.get_pagination_params(req)
images = self._image_service.detail(
context, filters=filters, marker=marker, limit=limit)
page_params = common.get_pagination_params(req)
images = self._image_service.detail(context, filters=filters,
**page_params)
builder = self.get_builder(req).build
return dict(images=[builder(image, detail=True) for image in images])

View File

@ -161,12 +161,12 @@ class PaginationParamsTest(test.TestCase):
def test_no_params(self):
""" Test no params. """
req = Request.blank('/')
self.assertEqual(common.get_pagination_params(req), (0, 0))
self.assertEqual(common.get_pagination_params(req), {})
def test_valid_marker(self):
""" Test valid marker param. """
req = Request.blank('/?marker=1')
self.assertEqual(common.get_pagination_params(req), (1, 0))
self.assertEqual(common.get_pagination_params(req), {'marker': 1})
def test_invalid_marker(self):
""" Test invalid marker param. """
@ -177,10 +177,16 @@ class PaginationParamsTest(test.TestCase):
def test_valid_limit(self):
""" Test valid limit param. """
req = Request.blank('/?limit=10')
self.assertEqual(common.get_pagination_params(req), (0, 10))
self.assertEqual(common.get_pagination_params(req), {'limit': 10})
def test_invalid_limit(self):
""" Test invalid limit param. """
req = Request.blank('/?limit=-2')
self.assertRaises(
webob.exc.HTTPBadRequest, common.get_pagination_params, req)
def test_valid_limit_and_marker(self):
""" Test valid limit and marker parameters. """
req = Request.blank('/?limit=20&marker=40')
self.assertEqual(common.get_pagination_params(req),
{'marker': 40, 'limit': 20})

View File

@ -802,7 +802,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase):
context = object()
filters = {'name': 'testname'}
image_service.index(
context, filters=filters, marker=0, limit=0).AndReturn([])
context, filters=filters).AndReturn([])
mocker.ReplayAll()
request = webob.Request.blank(
'/v1.1/images?name=testname')
@ -817,7 +817,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase):
context = object()
filters = {'status': 'ACTIVE'}
image_service.index(
context, filters=filters, marker=0, limit=0).AndReturn([])
context, filters=filters).AndReturn([])
mocker.ReplayAll()
request = webob.Request.blank(
'/v1.1/images?status=ACTIVE')
@ -832,7 +832,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase):
context = object()
filters = {'property-test': '3'}
image_service.index(
context, filters=filters, marker=0, limit=0).AndReturn([])
context, filters=filters).AndReturn([])
mocker.ReplayAll()
request = webob.Request.blank(
'/v1.1/images?property-test=3')
@ -847,7 +847,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase):
context = object()
filters = {'status': 'ACTIVE'}
image_service.index(
context, filters=filters, marker=0, limit=0).AndReturn([])
context, filters=filters).AndReturn([])
mocker.ReplayAll()
request = webob.Request.blank(
'/v1.1/images?status=ACTIVE&UNSUPPORTEDFILTER=testname')
@ -862,7 +862,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase):
context = object()
filters = {}
image_service.index(
context, filters=filters, marker=0, limit=0).AndReturn([])
context, filters=filters).AndReturn([])
mocker.ReplayAll()
request = webob.Request.blank(
'/v1.1/images')
@ -877,7 +877,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase):
context = object()
filters = {'name': 'testname'}
image_service.detail(
context, filters=filters, marker=0, limit=0).AndReturn([])
context, filters=filters).AndReturn([])
mocker.ReplayAll()
request = webob.Request.blank(
'/v1.1/images/detail?name=testname')
@ -892,7 +892,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase):
context = object()
filters = {'status': 'ACTIVE'}
image_service.detail(
context, filters=filters, marker=0, limit=0).AndReturn([])
context, filters=filters).AndReturn([])
mocker.ReplayAll()
request = webob.Request.blank(
'/v1.1/images/detail?status=ACTIVE')
@ -907,7 +907,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase):
context = object()
filters = {'property-test': '3'}
image_service.detail(
context, filters=filters, marker=0, limit=0).AndReturn([])
context, filters=filters).AndReturn([])
mocker.ReplayAll()
request = webob.Request.blank(
'/v1.1/images/detail?property-test=3')
@ -922,7 +922,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase):
context = object()
filters = {'status': 'ACTIVE'}
image_service.detail(
context, filters=filters, marker=0, limit=0).AndReturn([])
context, filters=filters).AndReturn([])
mocker.ReplayAll()
request = webob.Request.blank(
'/v1.1/images/detail?status=ACTIVE&UNSUPPORTEDFILTER=testname')
@ -937,7 +937,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase):
context = object()
filters = {}
image_service.detail(
context, filters=filters, marker=0, limit=0).AndReturn([])
context, filters=filters).AndReturn([])
mocker.ReplayAll()
request = webob.Request.blank(
'/v1.1/images/detail')