slight refactoring per jaypipes' suggestions; sort on get images calls is now created_at desc
This commit is contained in:
parent
70164edbd9
commit
d5d109b735
|
@ -313,25 +313,3 @@ class Serializer(object):
|
|||
node = doc.createTextNode(str(data))
|
||||
result.appendChild(node)
|
||||
return result
|
||||
|
||||
|
||||
def limit_collection(items, marker, limit):
|
||||
"""Return a set of items according to the requested marker and limit.
|
||||
|
||||
:param limit: maximum number of items to return
|
||||
:param marker: image id after which to start the page
|
||||
|
||||
"""
|
||||
start_index = 0
|
||||
if marker:
|
||||
start_index = -1
|
||||
for i, item in enumerate(items):
|
||||
if item['id'] == marker:
|
||||
start_index = i + 1
|
||||
break
|
||||
if start_index < 0:
|
||||
raise IndexError(start_index)
|
||||
|
||||
range_end = start_index + limit
|
||||
|
||||
return items[start_index:range_end]
|
||||
|
|
|
@ -44,7 +44,7 @@ class RegistryClient(BaseClient):
|
|||
port = port or self.DEFAULT_PORT
|
||||
super(RegistryClient, self).__init__(host, port, use_ssl)
|
||||
|
||||
def get_images(self, filters=None, marker=None, limit=1000):
|
||||
def get_images(self, filters=None, marker=None, limit=None):
|
||||
"""
|
||||
Returns a list of image id/name mappings from Registry
|
||||
|
||||
|
@ -52,15 +52,13 @@ class RegistryClient(BaseClient):
|
|||
:param marker: image id after which to start page
|
||||
:param limit: max number of images to return
|
||||
"""
|
||||
if filters != None:
|
||||
params = filters
|
||||
else:
|
||||
params = {}
|
||||
params = filters or {}
|
||||
|
||||
if marker != None:
|
||||
params['marker'] = marker
|
||||
|
||||
params['limit'] = limit
|
||||
if limit != None:
|
||||
params['limit'] = limit
|
||||
|
||||
action = "/images?%s" % urllib.urlencode(params)
|
||||
|
||||
|
@ -68,7 +66,7 @@ class RegistryClient(BaseClient):
|
|||
data = json.loads(res.read())['images']
|
||||
return data
|
||||
|
||||
def get_images_detailed(self, filters=None, marker=None, limit=1000):
|
||||
def get_images_detailed(self, filters=None, marker=None, limit=None):
|
||||
"""
|
||||
Returns a list of detailed image data mappings from Registry
|
||||
|
||||
|
@ -76,15 +74,13 @@ class RegistryClient(BaseClient):
|
|||
:param marker: image id after which to start page
|
||||
:param limit: max number of images to return
|
||||
"""
|
||||
if filters != None:
|
||||
params = filters
|
||||
else:
|
||||
params = {}
|
||||
params = filters or {}
|
||||
|
||||
if marker != None:
|
||||
params['marker'] = marker
|
||||
|
||||
params['limit'] = limit
|
||||
if limit != None:
|
||||
params['limit'] = limit
|
||||
|
||||
action = "/images/detail?%s" % urllib.urlencode(params)
|
||||
|
||||
|
|
|
@ -23,7 +23,7 @@ Defines interface for DB access
|
|||
|
||||
import logging
|
||||
|
||||
from sqlalchemy import create_engine
|
||||
from sqlalchemy import create_engine, desc
|
||||
from sqlalchemy.ext.declarative import declarative_base
|
||||
from sqlalchemy.orm import exc
|
||||
from sqlalchemy.orm import joinedload
|
||||
|
@ -149,8 +149,7 @@ def image_get_all_public(context, filters=None, marker=None, limit=None):
|
|||
:param limit: maximum number of images to return
|
||||
|
||||
"""
|
||||
if filters == None:
|
||||
filters = {}
|
||||
filters = filters or {}
|
||||
|
||||
session = get_session()
|
||||
query = session.query(models.Image).\
|
||||
|
@ -158,7 +157,8 @@ def image_get_all_public(context, filters=None, marker=None, limit=None):
|
|||
filter_by(deleted=_deleted(context)).\
|
||||
filter_by(is_public=True).\
|
||||
filter(models.Image.status != 'killed').\
|
||||
order_by(models.Image.id)
|
||||
order_by(desc(models.Image.created_at))
|
||||
|
||||
if 'size_min' in filters:
|
||||
query = query.filter(models.Image.size >= filters['size_min'])
|
||||
del filters['size_min']
|
||||
|
@ -174,7 +174,8 @@ def image_get_all_public(context, filters=None, marker=None, limit=None):
|
|||
query = query.filter(getattr(models.Image, k) == v)
|
||||
|
||||
if marker != None:
|
||||
query = query.filter(models.Image.id > marker)
|
||||
query = query.filter(models.Image.created_at <
|
||||
session.query(models.Image).filter_by(id=marker).one().created_at)
|
||||
|
||||
if limit != None:
|
||||
query = query.limit(limit)
|
||||
|
|
|
@ -39,6 +39,8 @@ DISPLAY_FIELDS_IN_INDEX = ['id', 'name', 'size',
|
|||
SUPPORTED_FILTERS = ['name', 'status', 'container_format', 'disk_format',
|
||||
'size_min', 'size_max']
|
||||
|
||||
MAX_ITEM_LIMIT = 25
|
||||
|
||||
|
||||
class Controller(wsgi.Controller):
|
||||
"""Controller for the reference implementation registry server"""
|
||||
|
@ -67,13 +69,13 @@ class Controller(wsgi.Controller):
|
|||
}
|
||||
|
||||
"""
|
||||
params = {'filters': self._get_filters(req)}
|
||||
|
||||
if 'limit' in req.str_params:
|
||||
params['limit'] = int(req.str_params.get('limit'))
|
||||
params = {
|
||||
'filters': self._get_filters(req),
|
||||
'limit': self._get_limit(req),
|
||||
}
|
||||
|
||||
if 'marker' in req.str_params:
|
||||
params['marker'] = int(req.str_params.get('marker'))
|
||||
params['marker'] = self._get_marker(req)
|
||||
|
||||
images = db_api.image_get_all_public(None, **params)
|
||||
|
||||
|
@ -97,13 +99,13 @@ class Controller(wsgi.Controller):
|
|||
all image model fields.
|
||||
|
||||
"""
|
||||
params = {'filters': self._get_filters(req)}
|
||||
|
||||
if 'limit' in req.str_params:
|
||||
params['limit'] = int(req.str_params.get('limit'))
|
||||
params = {
|
||||
'filters': self._get_filters(req),
|
||||
'limit': self._get_limit(req),
|
||||
}
|
||||
|
||||
if 'marker' in req.str_params:
|
||||
params['marker'] = int(req.str_params.get('marker'))
|
||||
params['marker'] = self._get_marker(req)
|
||||
|
||||
images = db_api.image_get_all_public(None, **params)
|
||||
|
||||
|
@ -132,19 +134,10 @@ class Controller(wsgi.Controller):
|
|||
|
||||
return filters
|
||||
|
||||
def _limit_collection(self, req, images):
|
||||
"""Return a single page of images."""
|
||||
#TODO(bcwaldon): make configurable through flag
|
||||
max_limit = 1000
|
||||
|
||||
def _get_limit(self, req):
|
||||
"""Parse a limit query param into something usable."""
|
||||
try:
|
||||
marker = int(req.str_params.get('marker', 0))
|
||||
except ValueError:
|
||||
raise webob.exc.HTTPBadRequest(_("marker param must "
|
||||
"be an integer"))
|
||||
|
||||
try:
|
||||
limit = int(req.str_params.get('limit', max_limit))
|
||||
limit = int(req.str_params.get('limit', MAX_ITEM_LIMIT))
|
||||
except ValueError:
|
||||
raise webob.exc.HTTPBadRequest(_("limit param must "
|
||||
"be an integer"))
|
||||
|
@ -153,12 +146,16 @@ class Controller(wsgi.Controller):
|
|||
raise webob.exc.HTTPBadRequest(_("limit param must "
|
||||
"be positive"))
|
||||
|
||||
limit = min(max_limit, limit)
|
||||
return min(MAX_ITEM_LIMIT, limit)
|
||||
|
||||
def _get_marker(self, req):
|
||||
"""Parse a marker query param into something usable."""
|
||||
try:
|
||||
return wsgi.limit_collection(images, marker, limit)
|
||||
except IndexError, exc:
|
||||
raise webob.exc.HTTPBadRequest(_("marker %s not found" % exc))
|
||||
marker = int(req.str_params.get('marker', None))
|
||||
except ValueError:
|
||||
raise webob.exc.HTTPBadRequest(_("marker param must "
|
||||
"be an integer"))
|
||||
return marker
|
||||
|
||||
def show(self, req, id):
|
||||
"""Return data about the given image id."""
|
||||
|
|
|
@ -1070,11 +1070,11 @@ class TestCurlApi(functional.FunctionalTest):
|
|||
|
||||
self.assertEqual(len(images['images']), 2)
|
||||
for image in images['images']:
|
||||
self.assertTrue(image['id'] < 3)
|
||||
self.assertTrue(image['id'] >= 2)
|
||||
|
||||
# 3. GET /images with marker 1
|
||||
# 3. GET /images with marker 2
|
||||
# Verify only two images were returned
|
||||
cmd = "curl http://0.0.0.0:%d/v1/images?marker=1" % api_port
|
||||
cmd = "curl http://0.0.0.0:%d/v1/images?marker=3" % api_port
|
||||
|
||||
exitcode, out, err = execute(cmd)
|
||||
|
||||
|
@ -1083,12 +1083,12 @@ class TestCurlApi(functional.FunctionalTest):
|
|||
|
||||
self.assertEqual(len(images['images']), 2)
|
||||
for image in images['images']:
|
||||
self.assertTrue(image['id'] >= 2)
|
||||
self.assertTrue(image['id'] <= 2)
|
||||
|
||||
# 4. GET /images with marker 1 and limit of 1
|
||||
# Verify only one image was returned with the correct id
|
||||
cmd = ("curl 'http://0.0.0.0:%d/v1/images?"
|
||||
"limit=1&marker=1'" % api_port)
|
||||
"limit=1&marker=2'" % api_port)
|
||||
|
||||
exitcode, out, err = execute(cmd)
|
||||
|
||||
|
@ -1096,12 +1096,12 @@ class TestCurlApi(functional.FunctionalTest):
|
|||
images = json.loads(out.strip())
|
||||
|
||||
self.assertEqual(len(images['images']), 1)
|
||||
self.assertEqual(images['images'][0]['id'], 2)
|
||||
self.assertEqual(images['images'][0]['id'], 1)
|
||||
|
||||
# 5. GET /images/detail with marker 1 and limit of 1
|
||||
# Verify only one image was returned with the correct id
|
||||
cmd = ("curl 'http://0.0.0.0:%d/v1/images/detail?"
|
||||
"limit=1&marker=1'" % api_port)
|
||||
"limit=1&marker=3'" % api_port)
|
||||
|
||||
exitcode, out, err = execute(cmd)
|
||||
|
||||
|
|
|
@ -421,6 +421,8 @@ def stub_out_registry_db_image_api(stubs):
|
|||
start_index = i + 1
|
||||
break
|
||||
|
||||
images = sorted(images, key=lambda i: i['created_at'])
|
||||
|
||||
return images[start_index:start_index + limit]
|
||||
|
||||
fake_datastore = FakeDatastore()
|
||||
|
|
Loading…
Reference in New Issue