Merge lp:~rackspace-titan/glance/api-results-ordering into lp:~hudson-openstack/glance/trunk

[Brian Waldon]

Added sort_key and sort_dir query params to apis and clients.

[Jay Pipes (jaypipes)]

Hi Brian!

Nice work, yet again!

Small suggestions:

For the SUPPORTED_PARAMS, and SUPPORTED_GET_PARAMS (in glance.client), could you make a glance.api.v1.images module-level function that returns these supported parameters? Either that, or make the constant at that module-level and import them into glance.client. That will ensure that future modifications to that constant do not have to make the modifications in two places. You did this for the SUPPORTED_SORT_KEYS constant in the registry, so might as well do it for SUPPORTED_PARAMS, too. :)

278 + _sort_dir = sort_dir or 'desc'
279 + sort_dir_func = {
280 + 'asc': asc,
281 + 'desc': desc,
282 + }[_sort_dir]

Instead of having the sort_dir param default to None, how about having it default to 'desc', in which case, the above can be simplified:

sort_dir_func = {
    'asc': asc,
    'desc': desc
}[sort_dir]

Same can be said for this:

284 + _sort_key = sort_key or 'created_at'
285 + sort_key_attr = getattr(models.Image, _sort_key)

Just make the sort_key parameter default to 'created_at', and shorten to:

sort_key_attr = getattr(models.Image, sort_key)

For this repeated block of code:

174 + params = kwargs.get('filters', {})
175 + params.update(self._extract_get_params(kwargs))

Go ahead and put the first line in the _extract_get_params() method and have _extract_get_params() return the entire parameter dictionary. DRY...

Cheers!
jay

review: Needs Fixing

[Brian Waldon (bcwaldon)]

I'll take care of your comments later today, Jay. I also just pushed some updates to the glance client docs. Missed that one spot last night.

[Brian Waldon (bcwaldon)]

> For the SUPPORTED_PARAMS, and SUPPORTED_GET_PARAMS (in glance.client), could
> you make a glance.api.v1.images module-level function that returns these
> supported parameters? Either that, or make the constant at that module-level
> and import them into glance.client. That will ensure that future modifications
> to that constant do not have to make the modifications in two places. You did
> this for the SUPPORTED_SORT_KEYS constant in the registry, so might as well do
> it for SUPPORTED_PARAMS, too. :)

I think I took care of this now. Can you double-check?

> 278 + _sort_dir = sort_dir or 'desc'
> 279 + sort_dir_func = {
> 280 + 'asc': asc,
> 281 + 'desc': desc,
> 282 + }[_sort_dir]
>
> Instead of having the sort_dir param default to None, how about having it
> default to 'desc', in which case, the above can be simplified:
>
>
> sort_dir_func = {
> 'asc': asc,
> 'desc': desc
> }[sort_dir]
>
> Same can be said for this:
>
> 284 + _sort_key = sort_key or 'created_at'
> 285 + sort_key_attr = getattr(models.Image, _sort_key)
>
> Just make the sort_key parameter default to 'created_at', and shorten to:
>
> sort_key_attr = getattr(models.Image, sort_key)

There are cases where None is being passed in explicitly. Is it okay if I leave it?

> For this repeated block of code:
>
> 174 + params = kwargs.get('filters', {})
> 175 + params.update(self._extract_get_params(kwargs))
>
> Go ahead and put the first line in the _extract_get_params() method and have
> _extract_get_params() return the entire parameter dictionary. DRY...

Got it.

[Jay Pipes (jaypipes)]

> I think I took care of this now. Can you double-check?

For the /glance/api/v1/images.py, looks good.

For the client, though:

161 +# parameters accepted in get_images* methods
162 +SUPPORTED_GET_PARAMS = ('marker', 'limit', 'sort_key', 'sort_dir')

I'd prefer you re-use the above constant in V1Client like so:

from glance.api.v1 import images as v1_images

and then move _extract_get_params() from the base Client class into the V1Client class, and reference v1_images.SUPPORTED_GET_PARAMS instead of the copied SUPPORTED_GET_PARAMS you have in the client.py file now... that make sense?

> > 278 + _sort_dir = sort_dir or 'desc'
> > 279 + sort_dir_func = {
> > 280 + 'asc': asc,
> > 281 + 'desc': desc,
> > 282 + }[_sort_dir]
> >
> > Instead of having the sort_dir param default to None, how about having it
> > default to 'desc', in which case, the above can be simplified:
> >
> >
> > sort_dir_func = {
> > 'asc': asc,
> > 'desc': desc
> > }[sort_dir]
> >
> > Same can be said for this:
> >
> > 284 + _sort_key = sort_key or 'created_at'
> > 285 + sort_key_attr = getattr(models.Image, _sort_key)
> >
> > Just make the sort_key parameter default to 'created_at', and shorten to:
> >
> > sort_key_attr = getattr(models.Image, sort_key)
>
> There are cases where None is being passed in explicitly. Is it okay if I
> leave it?

In what case is None being passed explicitly? Can we remove those cases and rely on default parameter values?

-jay

[Brian Waldon (bcwaldon)]

> > I think I took care of this now. Can you double-check?
>
> For the /glance/api/v1/images.py, looks good.
>
> For the client, though:
>
> 161 +# parameters accepted in get_images* methods
> 162 +SUPPORTED_GET_PARAMS = ('marker', 'limit', 'sort_key', 'sort_dir')
>
> I'd prefer you re-use the above constant in V1Client like so:
>
> from glance.api.v1 import images as v1_images
>
> and then move _extract_get_params() from the base Client class into the
> V1Client class, and reference v1_images.SUPPORTED_GET_PARAMS instead of the
> copied SUPPORTED_GET_PARAMS you have in the client.py file now... that make
> sense?

I had to move BaseClient out of glance/client.py and into glance/common/client.py. I was seeing some weird circular imports and that was the most direct solution I could think of.

I got all of the SUPPORTED_PARAMS stuff organized like you wanted. I ended up leaving the _extract_params function in BaseClient, as it is used by the registry client and the api client. I just have it passing in the allowed params.

> > > 278 + _sort_dir = sort_dir or 'desc'
> > > 279 + sort_dir_func = {
> > > 280 + 'asc': asc,
> > > 281 + 'desc': desc,
> > > 282 + }[_sort_dir]
> > >
> > > Instead of having the sort_dir param default to None, how about having it
> > > default to 'desc', in which case, the above can be simplified:
> > >
> > >
> > > sort_dir_func = {
> > > 'asc': asc,
> > > 'desc': desc
> > > }[sort_dir]
> > >
> > > Same can be said for this:
> > >
> > > 284 + _sort_key = sort_key or 'created_at'
> > > 285 + sort_key_attr = getattr(models.Image, _sort_key)
> > >
> > > Just make the sort_key parameter default to 'created_at', and shorten to:
> > >
> > > sort_key_attr = getattr(models.Image, sort_key)
> >
> > There are cases where None is being passed in explicitly. Is it okay if I
> > leave it?
>
> In what case is None being passed explicitly? Can we remove those cases and
> rely on default parameter values?

The function in registry/server.py that extracts the query params was returning None for keys that weren't provided. Now it filters out None. It's definitely cleaner to define the defaults in kwargs.

[lp:~rackspace-titan/glance/api-results-ordering]

148. By Brian Waldon on 2011-06-26

merging trunk

149. By Brian Waldon on 2011-06-26

restructuring client code

150. By Brian Waldon on 2011-06-26

adding base client module

151. By Brian Waldon on 2011-06-26

cleaning up None values being passed into images_get_all_public db call

[Jay Pipes (jaypipes)]

Wicked. Great work, Brian!

Teeny weeny style thing:

789 + """Extract necessary query parameters from http request.
790 +
791 + :param req: the Request object coming from the wsgi layer
792 + :retval dictionary of filters to apply to list of images
793 +
794 + """

Should be:

"""
Extract necessary query parameters from http request.

:param req: the Request object coming from the wsgi layer
:retval dictionary of filters to apply to list of images
"""

There are a number of docstrings in the test cases that also need fixing for that. Sorry for being picky :(

-jay

review: Needs Fixing

[lp:~rackspace-titan/glance/api-results-ordering]

152. By Brian Waldon on 2011-06-27

docstring

153. By Brian Waldon on 2011-06-27

reverting one import change; another docstring fix

[Brian Waldon (bcwaldon)]

I'm a little confused on what you want in the tests. Do you want me to change all of the comments like this:

"""Some really long dosctring that
wraps at 80 chars.

"""

to this:

"""
Some really long docstring that
wraps at 80 chars.
"""

I ask because the precedent is firmly set as the first style. I'm fine making changes, I just want to make sure I do it correctly.

[Brian Lamar (blamar)]

I think Nova and Glance have slightly differences in this aspect. Nova 'requires' a single line summary followed by a multi-line description:

"""This is the first line summary.

This is the multiple line description which in itself
can have multiple line breaks and stuff. Plus it has a newline
at the end of it all.

"""

If it's just one line then it has to be all on one line:

"""This is a one line summary that needs to be 80 char or less."""

But meh, I've had nightmares about this ;)

[Brian Waldon (bcwaldon)]

Brian: So how do we do it in glance?

[Jay Pipes (jaypipes)]

On Mon, Jun 27, 2011 at 1:19 PM, Brian Waldon
<email address hidden> wrote:
> Brian: So how do we do it in glance?

The way I wrote above...

"""
This is a description. It can go more than one
line long. It's silly to force a one-line description
and then a newline and a multi-line description. Sometimes that
just doesn't make much sense.

And having an extra newline at the end of the docstring doesn't make
much sense either. Oh, and I don't care if you end your sentences with
periods
"""

Cheers,
jay

[lp:~rackspace-titan/glance/api-results-ordering]

154. By Brian Waldon on 2011-06-28

docstrings\!

155. By Brian Waldon on 2011-06-28

fixing one last docstring

[Brian Waldon (bcwaldon)]

I updated all the doctrings I touched. Would you be cool with a follow-up branch to clean up docstrings across the project?

[Jay Pipes (jaypipes)]

But of course monsieur.
On Jun 28, 2011 8:52 AM, "Brian Waldon" <email address hidden> wrote:
> I updated all the doctrings I touched. Would you be cool with a follow-up
branch to clean up docstrings across the project?
> --
>
https://code.launchpad.net/~rackspace-titan/glance/api-results-ordering/+merge/65290
> You are reviewing the proposed merge of
lp:~rackspace-titan/glance/api-results-ordering into lp:glance.

[Jay Pipes (jaypipes)]

review: Approve

=== modified file 'bin/glance'
--- bin/glance	2011-04-27 18:00:49 +0000
+++ bin/glance	2011-06-28 13:52:02 +0000
@@ -194,7 +194,7 @@
                 print "Returned the following metadata for the new image:"
                 for k, v in sorted(image_meta.items()):
                     print " %(k)30s => %(v)s" % locals()
-        except client.ClientConnectionError, e:
+        except exception.ClientConnectionError, e:
             host = options.host
             port = options.port
             print ("Failed to connect to the Glance API server "

=== modified file 'doc/source/client.rst'
--- doc/source/client.rst	2011-05-25 20:03:00 +0000
+++ doc/source/client.rst	2011-06-28 13:52:02 +0000
@@ -113,7 +113,36 @@
   c = Client("glance.example.com", 9292)
 
   filters = {'status': 'saving', 'size_max': (5 * 1024 * 1024 * 1024)}
-  print c.get_images_detailed(filters)
+  print c.get_images_detailed(filters=filters)
+
+Sorting Images Returned via ``get_images()`` and ``get_images_detailed()``
+--------------------------------------------------------------------------
+
+Two parameters are available to sort the list of images returned by
+these methods.
+
+* ``sort_key: KEY``
+
+  Images can be ordered by the image attribute ``KEY``. Acceptable values:
+  ``id``, ``name``, ``status``, ``container_format``, ``disk_format``,
+  ``created_at`` (default) and ``updated_at``.
+
+* ``sort_dir: DIR``
+
+  The direction of the sort may be defined by ``DIR``. Accepted values:
+  ``asc`` for ascending or ``desc`` (default) for descending.
+
+The following example will return a list of images sorted alphabetically
+by name in ascending order.
+
+.. code-block:: python
+
+  from glance.client import Client
+
+  c = Client("glance.example.com", 9292)
+
+  print c.get_images(sort_key='name', sort_dir='asc')
+
 
 Requesting Detailed Metadata on a Specific Image
 ------------------------------------------------

=== modified file 'doc/source/glanceapi.rst'
--- doc/source/glanceapi.rst	2011-05-26 12:53:48 +0000
+++ doc/source/glanceapi.rst	2011-06-28 13:52:02 +0000
@@ -129,6 +129,20 @@
 
   Filters images having a ``size`` attribute less than or equal to ``BYTES``
 
+These two resources also accept sort parameters:
+
+* ``sort_key=KEY``
+
+  Results will be ordered by the specified image attribute ``KEY``. Accepted
+  values include ``id``, ``name``, ``status``, ``disk_format``,
+  ``container_format``, ``size``, ``created_at`` (default) and ``updated_at``.
+
+* ``sort_dir=DIR``
+
+  Results will be sorted in the direction ``DIR``. Accepted values are ``asc``
+  for ascending or ``desc`` (default) for descending.
+
+
 Requesting Detailed Metadata on a Specific Image
 ------------------------------------------------
 

=== modified file 'doc/source/registries.rst'
--- doc/source/registries.rst	2011-05-26 12:53:48 +0000
+++ doc/source/registries.rst	2011-06-28 13:52:02 +0000
@@ -83,6 +83,20 @@
 
   Filters images having a ``size`` attribute less than or equal to ``BYTES``
 
+These two resources also accept sort parameters:
+
+* ``sort_key=KEY``
+
+  Results will be ordered by the specified image attribute ``KEY``. Accepted
+  values include ``id``, ``name``, ``status``, ``disk_format``,
+  ``container_format``, ``size``, ``created_at`` (default) and ``updated_at``.
+
+* ``sort_dir=DIR``
+
+  Results will be sorted in the direction ``DIR``. Accepted values are ``asc``
+  for ascending or ``desc`` (default) for descending.
+  
+
 ``POST /images``
 ----------------
 

=== modified file 'glance/api/v1/images.py'
--- glance/api/v1/images.py	2011-06-27 14:37:40 +0000
+++ glance/api/v1/images.py	2011-06-28 13:52:02 +0000
@@ -45,6 +45,8 @@
 SUPPORTED_FILTERS = ['name', 'status', 'container_format', 'disk_format',
                      'size_min', 'size_max']
 
+SUPPORTED_PARAMS = ('limit', 'marker', 'sort_key', 'sort_dir')
+
 
 class Controller(object):
 
@@ -92,14 +94,7 @@
                  'size': <SIZE>}, ...
             ]}
         """
-        params = {'filters': self._get_filters(req)}
-
-        if 'limit' in req.str_params:
-            params['limit'] = req.str_params.get('limit')
-
-        if 'marker' in req.str_params:
-            params['marker'] = req.str_params.get('marker')
-
+        params = self._get_query_params(req)
         images = registry.get_images_list(self.options, **params)
         return dict(images=images)
 
@@ -125,17 +120,23 @@
                  'properties': {'distro': 'Ubuntu 10.04 LTS', ...}}, ...
             ]}
         """
-        params = {'filters': self._get_filters(req)}
-
-        if 'limit' in req.str_params:
-            params['limit'] = req.str_params.get('limit')
-
-        if 'marker' in req.str_params:
-            params['marker'] = req.str_params.get('marker')
-
+        params = self._get_query_params(req)
         images = registry.get_images_detail(self.options, **params)
         return dict(images=images)
 
+    def _get_query_params(self, req):
+        """
+        Extracts necessary query params from request.
+
+        :param req: the WSGI Request object
+        :retval dict of parameters that can be used by registry client
+        """
+        params = {'filters': self._get_filters(req)}
+        for PARAM in SUPPORTED_PARAMS:
+            if PARAM in req.str_params:
+                params[PARAM] = req.str_params.get(PARAM)
+        return params
+
     def _get_filters(self, req):
         """
         Return a dictionary of query param filters from the request

=== modified file 'glance/client.py'
--- glance/client.py	2011-05-31 13:21:16 +0000
+++ glance/client.py	2011-06-28 13:52:02 +0000
@@ -19,172 +19,17 @@
 Client classes for callers of a Glance system
 """
 
-import httplib
 import json
-import logging
-import urlparse
-import socket
-import sys
-import urllib
 
+from glance.api.v1 import images as v1_images
+from glance.common import client as base_client
+from glance.common import exception
 from glance import utils
-from glance.common import exception
 
 #TODO(jaypipes) Allow a logger param for client classes
 
 
-class ClientConnectionError(Exception):
-    """Error resulting from a client connecting to a server"""
-    pass
-
-
-class ImageBodyIterator(object):
-
-    """
-    A class that acts as an iterator over an image file's
-    chunks of data.  This is returned as part of the result
-    tuple from `glance.client.Client.get_image`
-    """
-
-    CHUNKSIZE = 65536
-
-    def __init__(self, response):
-        """
-        Constructs the object from an HTTPResponse object
-        """
-        self.response = response
-
-    def __iter__(self):
-        """
-        Exposes an iterator over the chunks of data in the
-        image file.
-        """
-        while True:
-            chunk = self.response.read(ImageBodyIterator.CHUNKSIZE)
-            if chunk:
-                yield chunk
-            else:
-                break
-
-
-class BaseClient(object):
-
-    """A base client class"""
-
-    CHUNKSIZE = 65536
-
-    def __init__(self, host, port, use_ssl):
-        """
-        Creates a new client to some service.
-
-        :param host: The host where service resides
-        :param port: The port where service resides
-        :param use_ssl: Should we use HTTPS?
-        """
-        self.host = host
-        self.port = port
-        self.use_ssl = use_ssl
-        self.connection = None
-
-    def get_connection_type(self):
-        """
-        Returns the proper connection type
-        """
-        if self.use_ssl:
-            return httplib.HTTPSConnection
-        else:
-            return httplib.HTTPConnection
-
-    def do_request(self, method, action, body=None, headers=None,
-                   params=None):
-        """
-        Connects to the server and issues a request.  Handles converting
-        any returned HTTP error status codes to OpenStack/Glance exceptions
-        and closing the server connection. Returns the result data, or
-        raises an appropriate exception.
-
-        :param method: HTTP method ("GET", "POST", "PUT", etc...)
-        :param action: part of URL after root netloc
-        :param body: string of data to send, or None (default)
-        :param headers: mapping of key/value pairs to add as headers
-        :param params: dictionary of key/value pairs to add to append
-                             to action
-
-        :note
-
-        If the body param has a read attribute, and method is either
-        POST or PUT, this method will automatically conduct a chunked-transfer
-        encoding and use the body as a file object, transferring chunks
-        of data using the connection's send() method. This allows large
-        objects to be transferred efficiently without buffering the entire
-        body in memory.
-        """
-        if type(params) is dict:
-            action += '?' + urllib.urlencode(params)
-
-        try:
-            connection_type = self.get_connection_type()
-            headers = headers or {}
-            c = connection_type(self.host, self.port)
-
-            # Do a simple request or a chunked request, depending
-            # on whether the body param is a file-like object and
-            # the method is PUT or POST
-            if hasattr(body, 'read') and method.lower() in ('post', 'put'):
-                # Chunk it, baby...
-                c.putrequest(method, action)
-
-                for header, value in headers.items():
-                    c.putheader(header, value)
-                c.putheader('Transfer-Encoding', 'chunked')
-                c.endheaders()
-
-                chunk = body.read(self.CHUNKSIZE)
-                while chunk:
-                    c.send('%x\r\n%s\r\n' % (len(chunk), chunk))
-                    chunk = body.read(self.CHUNKSIZE)
-                c.send('0\r\n\r\n')
-            else:
-                # Simple request...
-                c.request(method, action, body, headers)
-            res = c.getresponse()
-            status_code = self.get_status_code(res)
-            if status_code in (httplib.OK,
-                               httplib.CREATED,
-                               httplib.ACCEPTED,
-                               httplib.NO_CONTENT):
-                return res
-            elif status_code == httplib.UNAUTHORIZED:
-                raise exception.NotAuthorized
-            elif status_code == httplib.FORBIDDEN:
-                raise exception.NotAuthorized
-            elif status_code == httplib.NOT_FOUND:
-                raise exception.NotFound
-            elif status_code == httplib.CONFLICT:
-                raise exception.Duplicate(res.read())
-            elif status_code == httplib.BAD_REQUEST:
-                raise exception.Invalid(res.read())
-            elif status_code == httplib.INTERNAL_SERVER_ERROR:
-                raise Exception("Internal Server error: %s" % res.read())
-            else:
-                raise Exception("Unknown error occurred! %s" % res.read())
-
-        except (socket.error, IOError), e:
-            raise ClientConnectionError("Unable to connect to "
-                                        "server. Got error: %s" % e)
-
-    def get_status_code(self, response):
-        """
-        Returns the integer status code from the response, which
-        can be either a Webob.Response (used in testing) or httplib.Response
-        """
-        if hasattr(response, 'status_int'):
-            return response.status_int
-        else:
-            return response.status
-
-
-class V1Client(BaseClient):
+class V1Client(base_client.BaseClient):
 
     """Main client class for accessing Glance resources"""
 
@@ -209,7 +54,7 @@
         return super(V1Client, self).do_request(method, action, body,
                                                 headers, params)
 
-    def get_images(self, filters=None, marker=None, limit=None):
+    def get_images(self, **kwargs):
         """
         Returns a list of image id/name mappings from Registry
 
@@ -217,18 +62,15 @@
                         collection of images should be filtered
         :param marker: id after which to start the page of images
         :param limit: maximum number of items to return
+        :param sort_key: results will be ordered by this image attribute
+        :param sort_dir: direction in which to to order results (asc, desc)
         """
-
-        params = filters or {}
-        if marker:
-            params['marker'] = marker
-        if limit:
-            params['limit'] = limit
+        params = self._extract_params(kwargs, v1_images.SUPPORTED_PARAMS)
         res = self.do_request("GET", "/images", params=params)
         data = json.loads(res.read())['images']
         return data
 
-    def get_images_detailed(self, filters=None, marker=None, limit=None):
+    def get_images_detailed(self, **kwargs):
         """
         Returns a list of detailed image data mappings from Registry
 
@@ -236,13 +78,11 @@
                         collection of images should be filtered
         :param marker: id after which to start the page of images
         :param limit: maximum number of items to return
+        :param sort_key: results will be ordered by this image attribute
+        :param sort_dir: direction in which to to order results (asc, desc)
         """
 
-        params = filters or {}
-        if marker:
-            params['marker'] = marker
-        if limit:
-            params['limit'] = limit
+        params = self._extract_params(kwargs, v1_images.SUPPORTED_PARAMS)
         res = self.do_request("GET", "/images/detail", params=params)
         data = json.loads(res.read())['images']
         return data
@@ -260,7 +100,7 @@
         res = self.do_request("GET", "/images/%s" % image_id)
 
         image = utils.get_image_meta_from_headers(res)
-        return image, ImageBodyIterator(res)
+        return image, base_client.ImageBodyIterator(res)
 
     def get_image_meta(self, image_id):
         """

=== added file 'glance/common/client.py'
--- glance/common/client.py	1970-01-01 00:00:00 +0000
+++ glance/common/client.py	2011-06-28 13:52:02 +0000
@@ -0,0 +1,169 @@
+import httplib
+import logging
+import socket
+import urllib
+
+from glance.common import exception
+
+
+class ImageBodyIterator(object):
+
+    """
+    A class that acts as an iterator over an image file's
+    chunks of data.  This is returned as part of the result
+    tuple from `glance.client.Client.get_image`
+    """
+
+    CHUNKSIZE = 65536
+
+    def __init__(self, response):
+        """
+        Constructs the object from an HTTPResponse object
+        """
+        self.response = response
+
+    def __iter__(self):
+        """
+        Exposes an iterator over the chunks of data in the
+        image file.
+        """
+        while True:
+            chunk = self.response.read(ImageBodyIterator.CHUNKSIZE)
+            if chunk:
+                yield chunk
+            else:
+                break
+
+
+class BaseClient(object):
+
+    """A base client class"""
+
+    CHUNKSIZE = 65536
+
+    def __init__(self, host, port, use_ssl):
+        """
+        Creates a new client to some service.
+
+        :param host: The host where service resides
+        :param port: The port where service resides
+        :param use_ssl: Should we use HTTPS?
+        """
+        self.host = host
+        self.port = port
+        self.use_ssl = use_ssl
+        self.connection = None
+
+    def get_connection_type(self):
+        """
+        Returns the proper connection type
+        """
+        if self.use_ssl:
+            return httplib.HTTPSConnection
+        else:
+            return httplib.HTTPConnection
+
+    def do_request(self, method, action, body=None, headers=None,
+                   params=None):
+        """
+        Connects to the server and issues a request.  Handles converting
+        any returned HTTP error status codes to OpenStack/Glance exceptions
+        and closing the server connection. Returns the result data, or
+        raises an appropriate exception.
+
+        :param method: HTTP method ("GET", "POST", "PUT", etc...)
+        :param action: part of URL after root netloc
+        :param body: string of data to send, or None (default)
+        :param headers: mapping of key/value pairs to add as headers
+        :param params: dictionary of key/value pairs to add to append
+                             to action
+
+        :note
+
+        If the body param has a read attribute, and method is either
+        POST or PUT, this method will automatically conduct a chunked-transfer
+        encoding and use the body as a file object, transferring chunks
+        of data using the connection's send() method. This allows large
+        objects to be transferred efficiently without buffering the entire
+        body in memory.
+        """
+        if type(params) is dict:
+            action += '?' + urllib.urlencode(params)
+
+        try:
+            connection_type = self.get_connection_type()
+            headers = headers or {}
+            c = connection_type(self.host, self.port)
+
+            # Do a simple request or a chunked request, depending
+            # on whether the body param is a file-like object and
+            # the method is PUT or POST
+            if hasattr(body, 'read') and method.lower() in ('post', 'put'):
+                # Chunk it, baby...
+                c.putrequest(method, action)
+
+                for header, value in headers.items():
+                    c.putheader(header, value)
+                c.putheader('Transfer-Encoding', 'chunked')
+                c.endheaders()
+
+                chunk = body.read(self.CHUNKSIZE)
+                while chunk:
+                    c.send('%x\r\n%s\r\n' % (len(chunk), chunk))
+                    chunk = body.read(self.CHUNKSIZE)
+                c.send('0\r\n\r\n')
+            else:
+                # Simple request...
+                c.request(method, action, body, headers)
+            res = c.getresponse()
+            status_code = self.get_status_code(res)
+            if status_code in (httplib.OK,
+                               httplib.CREATED,
+                               httplib.ACCEPTED,
+                               httplib.NO_CONTENT):
+                return res
+            elif status_code == httplib.UNAUTHORIZED:
+                raise exception.NotAuthorized
+            elif status_code == httplib.FORBIDDEN:
+                raise exception.NotAuthorized
+            elif status_code == httplib.NOT_FOUND:
+                raise exception.NotFound
+            elif status_code == httplib.CONFLICT:
+                raise exception.Duplicate(res.read())
+            elif status_code == httplib.BAD_REQUEST:
+                raise exception.Invalid(res.read())
+            elif status_code == httplib.INTERNAL_SERVER_ERROR:
+                raise Exception("Internal Server error: %s" % res.read())
+            else:
+                raise Exception("Unknown error occurred! %s" % res.read())
+
+        except (socket.error, IOError), e:
+            raise exception.ClientConnectionError("Unable to connect to "
+                                                  "server. Got error: %s" % e)
+
+    def get_status_code(self, response):
+        """
+        Returns the integer status code from the response, which
+        can be either a Webob.Response (used in testing) or httplib.Response
+        """
+        if hasattr(response, 'status_int'):
+            return response.status_int
+        else:
+            return response.status
+
+    def _extract_params(self, actual_params, allowed_params):
+        """
+        Extract a subset of keys from a dictionary. The filters key
+        will also be extracted, and each of its values will be returned
+        as an individual param.
+
+        :param actual_params: dict of keys to filter
+        :param allowed_params: list of keys that 'actual_params' will be
+                               reduced to
+        :retval subset of 'params' dict
+        """
+        result = actual_params.get('filters', {})
+        for allowed_param in allowed_params:
+            if allowed_param in actual_params:
+                result[allowed_param] = actual_params[allowed_param]
+        return result

=== modified file 'glance/common/exception.py'
--- glance/common/exception.py	2011-06-15 14:47:00 +0000
+++ glance/common/exception.py	2011-06-28 13:52:02 +0000
@@ -83,6 +83,11 @@
     pass
 
 
+class ClientConnectionError(Exception):
+    """Error resulting from a client connecting to a server"""
+    pass
+
+
 def wrap_exception(f):
     def _wrap(*args, **kw):
         try:

=== modified file 'glance/registry/client.py'
--- glance/registry/client.py	2011-06-11 13:47:47 +0000
+++ glance/registry/client.py	2011-06-28 13:52:02 +0000
@@ -23,7 +23,8 @@
 import json
 import urllib
 
-from glance.client import BaseClient
+from glance.common.client import BaseClient
+from glance.registry import server
 
 
 class RegistryClient(BaseClient):
@@ -44,42 +45,32 @@
         port = port or self.DEFAULT_PORT
         super(RegistryClient, self).__init__(host, port, use_ssl)
 
-    def get_images(self, filters=None, marker=None, limit=None):
+    def get_images(self, **kwargs):
         """
         Returns a list of image id/name mappings from Registry
 
         :param filters: dict of keys & expected values to filter results
         :param marker: image id after which to start page
         :param limit: max number of images to return
+        :param sort_key: results will be ordered by this image attribute
+        :param sort_dir: direction in which to to order results (asc, desc)
         """
-        params = filters or {}
-
-        if marker != None:
-            params['marker'] = marker
-
-        if limit != None:
-            params['limit'] = limit
-
+        params = self._extract_params(kwargs, server.SUPPORTED_PARAMS)
         res = self.do_request("GET", "/images", params=params)
         data = json.loads(res.read())['images']
         return data
 
-    def get_images_detailed(self, filters=None, marker=None, limit=None):
+    def get_images_detailed(self, **kwargs):
         """
         Returns a list of detailed image data mappings from Registry
 
         :param filters: dict of keys & expected values to filter results
         :param marker: image id after which to start page
         :param limit: max number of images to return
+        :param sort_key: results will be ordered by this image attribute
+        :param sort_dir: direction in which to to order results (asc, desc)
         """
-        params = filters or {}
-
-        if marker != None:
-            params['marker'] = marker
-
-        if limit != None:
-            params['limit'] = limit
-
+        params = self._extract_params(kwargs, server.SUPPORTED_PARAMS)
         res = self.do_request("GET", "/images/detail", params=params)
         data = json.loads(res.read())['images']
         return data

=== modified file 'glance/registry/db/api.py'
--- glance/registry/db/api.py	2011-05-31 19:32:17 +0000
+++ glance/registry/db/api.py	2011-06-28 13:52:02 +0000
@@ -23,7 +23,7 @@
 
 import logging
 
-from sqlalchemy import create_engine, desc
+from sqlalchemy import asc, create_engine, desc
 from sqlalchemy.ext.declarative import declarative_base
 from sqlalchemy.orm import exc
 from sqlalchemy.orm import joinedload
@@ -129,7 +129,8 @@
         raise exception.NotFound("No image found with ID %s" % image_id)
 
 
-def image_get_all_public(context, filters=None, marker=None, limit=None):
+def image_get_all_public(context, filters=None, marker=None, limit=None,
+                         sort_key='created_at', sort_dir='desc'):
     """Get all public images that match zero or more filters.
 
     :param filters: dict of filter keys and values. If a 'properties'
@@ -137,7 +138,8 @@
                     filters on the image properties attribute
     :param marker: image id after which to start page
     :param limit: maximum number of images to return
-
+    :param sort_key: image attribute by which results should be sorted
+    :param sort_dir: direction in which results should be sorted (asc, desc)
     """
     filters = filters or {}
 
@@ -146,9 +148,17 @@
                    options(joinedload(models.Image.properties)).\
                    filter_by(deleted=_deleted(context)).\
                    filter_by(is_public=True).\
-                   filter(models.Image.status != 'killed').\
-                   order_by(desc(models.Image.created_at)).\
-                   order_by(desc(models.Image.id))
+                   filter(models.Image.status != 'killed')
+
+    sort_dir_func = {
+        'asc': asc,
+        'desc': desc,
+    }[sort_dir]
+
+    sort_key_attr = getattr(models.Image, sort_key)
+
+    query = query.order_by(sort_dir_func(sort_key_attr)).\
+                  order_by(sort_dir_func(models.Image.id))
 
     if 'size_min' in filters:
         query = query.filter(models.Image.size >= filters['size_min'])

=== modified file 'glance/registry/server.py'
--- glance/registry/server.py	2011-06-15 14:47:00 +0000
+++ glance/registry/server.py	2011-06-28 13:52:02 +0000
@@ -39,8 +39,15 @@
 SUPPORTED_FILTERS = ['name', 'status', 'container_format', 'disk_format',
                      'size_min', 'size_max']
 
+SUPPORTED_SORT_KEYS = ('name', 'status', 'container_format', 'disk_format',
+                       'size', 'id', 'created_at', 'updated_at')
+
+SUPPORTED_SORT_DIRS = ('asc', 'desc')
+
 MAX_ITEM_LIMIT = 25
 
+SUPPORTED_PARAMS = ('limit', 'marker', 'sort_key', 'sort_dir')
+
 
 class Controller(object):
     """Controller for the reference implementation registry server"""
@@ -69,14 +76,7 @@
             }
 
         """
-        params = {
-            'filters': self._get_filters(req),
-            'limit': self._get_limit(req),
-        }
-
-        if 'marker' in req.str_params:
-            params['marker'] = self._get_marker(req)
-
+        params = self._get_query_params(req)
         images = db_api.image_get_all_public(None, **params)
 
         results = []
@@ -99,19 +99,33 @@
         all image model fields.
 
         """
-        params = {
-            'filters': self._get_filters(req),
-            'limit': self._get_limit(req),
-        }
-
-        if 'marker' in req.str_params:
-            params['marker'] = self._get_marker(req)
-
+        params = self._get_query_params(req)
         images = db_api.image_get_all_public(None, **params)
 
         image_dicts = [make_image_dict(i) for i in images]
         return dict(images=image_dicts)
 
+    def _get_query_params(self, req):
+        """
+        Extract necessary query parameters from http request.
+
+        :param req: the Request object coming from the wsgi layer
+        :retval dictionary of filters to apply to list of images
+        """
+        params = {
+            'filters': self._get_filters(req),
+            'limit': self._get_limit(req),
+            'sort_key': self._get_sort_key(req),
+            'sort_dir': self._get_sort_dir(req),
+            'marker': self._get_marker(req),
+        }
+
+        for key, value in params.items():
+            if value is None:
+                del params[key]
+
+        return params
+
     def _get_filters(self, req):
         """Return a dictionary of query param filters from the request
 
@@ -148,12 +162,35 @@
 
     def _get_marker(self, req):
         """Parse a marker query param into something usable."""
+        marker = req.str_params.get('marker', None)
+
+        if marker is None:
+            return None
+
         try:
-            marker = int(req.str_params.get('marker', None))
+            marker = int(marker)
         except ValueError:
             raise exc.HTTPBadRequest("marker param must be an integer")
         return marker
 
+    def _get_sort_key(self, req):
+        """Parse a sort key query param from the request object."""
+        sort_key = req.str_params.get('sort_key', None)
+        if sort_key is not None and sort_key not in SUPPORTED_SORT_KEYS:
+            _keys = ', '.join(SUPPORTED_SORT_KEYS)
+            msg = "Unsupported sort_key. Acceptable values: %s" % (_keys,)
+            raise exc.HTTPBadRequest(explanation=msg)
+        return sort_key
+
+    def _get_sort_dir(self, req):
+        """Parse a sort direction query param from the request object."""
+        sort_dir = req.str_params.get('sort_dir', None)
+        if sort_dir is not None and sort_dir not in SUPPORTED_SORT_DIRS:
+            _keys = ', '.join(SUPPORTED_SORT_DIRS)
+            msg = "Unsupported sort_dir. Acceptable values: %s" % (_keys,)
+            raise exc.HTTPBadRequest(explanation=msg)
+        return sort_dir
+
     def show(self, req, id):
         """Return data about the given image id."""
         try:

=== modified file 'tests/functional/test_curl_api.py'
--- tests/functional/test_curl_api.py	2011-06-22 14:19:33 +0000
+++ tests/functional/test_curl_api.py	2011-06-28 13:52:02 +0000
@@ -1117,3 +1117,118 @@
         self.assertEqual(int(images[0]['id']), 2)
 
         self.stop_servers()
+
+    def test_ordered_images(self):
+        """
+        Set up three test images and ensure each query param filter works
+        """
+        self.cleanup()
+        self.start_servers()
+
+        api_port = self.api_port
+        registry_port = self.registry_port
+
+        # 0. GET /images
+        # Verify no public images
+        cmd = "curl http://0.0.0.0:%d/v1/images" % api_port
+
+        exitcode, out, err = execute(cmd)
+
+        self.assertEqual(0, exitcode)
+        self.assertEqual('{"images": []}', out.strip())
+
+        # 1. POST /images with three public images with various attributes
+        cmd = ("curl -i -X POST "
+               "-H 'Expect: ' "  # Necessary otherwise sends 100 Continue
+               "-H 'X-Image-Meta-Name: Image1' "
+               "-H 'X-Image-Meta-Status: active' "
+               "-H 'X-Image-Meta-Container-Format: ovf' "
+               "-H 'X-Image-Meta-Disk-Format: vdi' "
+               "-H 'X-Image-Meta-Size: 19' "
+               "-H 'X-Image-Meta-Is-Public: True' "
+               "http://0.0.0.0:%d/v1/images") % api_port
+
+        exitcode, out, err = execute(cmd)
+        self.assertEqual(0, exitcode)
+
+        lines = out.split("\r\n")
+        status_line = lines[0]
+
+        self.assertEqual("HTTP/1.1 201 Created", status_line)
+
+        cmd = ("curl -i -X POST "
+               "-H 'Expect: ' "  # Necessary otherwise sends 100 Continue
+               "-H 'X-Image-Meta-Name: ASDF' "
+               "-H 'X-Image-Meta-Status: active' "
+               "-H 'X-Image-Meta-Container-Format: bare' "
+               "-H 'X-Image-Meta-Disk-Format: iso' "
+               "-H 'X-Image-Meta-Size: 2' "
+               "-H 'X-Image-Meta-Is-Public: True' "
+               "http://0.0.0.0:%d/v1/images") % api_port
+
+        exitcode, out, err = execute(cmd)
+        self.assertEqual(0, exitcode)
+
+        lines = out.split("\r\n")
+        status_line = lines[0]
+
+        self.assertEqual("HTTP/1.1 201 Created", status_line)
+        cmd = ("curl -i -X POST "
+               "-H 'Expect: ' "  # Necessary otherwise sends 100 Continue
+               "-H 'X-Image-Meta-Name: XYZ' "
+               "-H 'X-Image-Meta-Status: saving' "
+               "-H 'X-Image-Meta-Container-Format: ami' "
+               "-H 'X-Image-Meta-Disk-Format: ami' "
+               "-H 'X-Image-Meta-Size: 5' "
+               "-H 'X-Image-Meta-Is-Public: True' "
+               "http://0.0.0.0:%d/v1/images") % api_port
+
+        exitcode, out, err = execute(cmd)
+        self.assertEqual(0, exitcode)
+
+        lines = out.split("\r\n")
+        status_line = lines[0]
+
+        self.assertEqual("HTTP/1.1 201 Created", status_line)
+
+        # 2. GET /images with no query params
+        # Verify three public images sorted by created_at desc
+        cmd = "curl http://0.0.0.0:%d/v1/images" % api_port
+
+        exitcode, out, err = execute(cmd)
+
+        self.assertEqual(0, exitcode)
+        images = json.loads(out.strip())['images']
+
+        self.assertEqual(len(images), 3)
+        self.assertEqual(images[0]['id'], 3)
+        self.assertEqual(images[1]['id'], 2)
+        self.assertEqual(images[2]['id'], 1)
+
+        # 3. GET /images sorted by name asc
+        params = 'sort_key=name&sort_dir=asc'
+        cmd = "curl 'http://0.0.0.0:%d/v1/images?%s'" % (api_port, params)
+
+        exitcode, out, err = execute(cmd)
+
+        self.assertEqual(0, exitcode)
+        images = json.loads(out.strip())['images']
+
+        self.assertEqual(len(images), 3)
+        self.assertEqual(images[0]['id'], 2)
+        self.assertEqual(images[1]['id'], 1)
+        self.assertEqual(images[2]['id'], 3)
+
+        # 4. GET /images sorted by size desc
+        params = 'sort_key=size&sort_dir=desc'
+        cmd = "curl 'http://0.0.0.0:%d/v1/images?%s'" % (api_port, params)
+
+        exitcode, out, err = execute(cmd)
+
+        self.assertEqual(0, exitcode)
+        images = json.loads(out.strip())['images']
+
+        self.assertEqual(len(images), 3)
+        self.assertEqual(images[0]['id'], 1)
+        self.assertEqual(images[1]['id'], 3)
+        self.assertEqual(images[2]['id'], 2)

=== modified file 'tests/stubs.py'
--- tests/stubs.py	2011-05-31 19:32:17 +0000
+++ tests/stubs.py	2011-06-28 13:52:02 +0000
@@ -28,6 +28,7 @@
 import stubout
 import webob
 
+import glance.common.client
 from glance.common import exception
 from glance.registry import server as rserver
 from glance.api import v1 as server
@@ -254,9 +255,9 @@
         for i in self.response.app_iter:
             yield i
 
-    stubs.Set(glance.client.BaseClient, 'get_connection_type',
+    stubs.Set(glance.common.client.BaseClient, 'get_connection_type',
               fake_get_connection_type)
-    stubs.Set(glance.client.ImageBodyIterator, '__iter__',
+    stubs.Set(glance.common.client.ImageBodyIterator, '__iter__',
               fake_image_iter)
 
 
@@ -388,8 +389,8 @@
             else:
                 return images[0]
 
-        def image_get_all_public(self, _context, filters=None,
-                                 marker=None, limit=1000):
+        def image_get_all_public(self, _context, filters=None, marker=None,
+                                 limit=1000, sort_key=None, sort_dir=None):
             images = [f for f in self.images if f['is_public'] == True]
 
             if 'size_min' in filters:
@@ -414,16 +415,24 @@
             for k, v in filters.items():
                 images = [f for f in images if f[k] == v]
 
+            # sorted func expects func that compares in descending order
             def image_cmp(x, y):
-                if x['created_at'] > y['created_at']:
-                    return 1
-                elif x['created_at'] == y['created_at']:
+                _sort_dir = sort_dir or 'desc'
+                multiplier = {
+                    'asc': -1,
+                    'desc': 1,
+                }[_sort_dir]
+
+                _sort_key = sort_key or 'created_at'
+                if x[_sort_key] > y[_sort_key]:
+                    return 1 * multiplier
+                elif x[_sort_key] == y[_sort_key]:
                     if x['id'] > y['id']:
-                        return 1
+                        return 1 * multiplier
                     else:
-                        return -1
+                        return -1 * multiplier
                 else:
-                    return -1
+                    return -1 * multiplier
 
             images = sorted(images, cmp=image_cmp)
             images.reverse()

=== modified file 'tests/unit/test_api.py'
--- tests/unit/test_api.py	2011-06-24 20:57:02 +0000
+++ tests/unit/test_api.py	2011-06-28 13:52:02 +0000
@@ -285,6 +285,398 @@
         for image in images:
             self.assertEqual('new name! #123', image['name'])
 
+    def test_get_index_sort_default_created_at_desc(self):
+        """
+        Tests that the /images registry API returns list of
+        public images that conforms to a default sort key/dir
+        """
+        time1 = datetime.datetime.utcnow() + datetime.timedelta(seconds=5)
+        time2 = datetime.datetime.utcnow()
+
+        extra_fixture = {'id': 3,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'new name! #123',
+                         'size': 19,
+                         'checksum': None,
+                         'created_at': time1}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        extra_fixture = {'id': 4,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'new name! #123',
+                         'size': 20,
+                         'checksum': None,
+                         'created_at': time1}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        extra_fixture = {'id': 5,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'new name! #123',
+                         'size': 20,
+                         'checksum': None,
+                         'created_at': time2}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        req = webob.Request.blank('/images')
+        res = req.get_response(self.api)
+        res_dict = json.loads(res.body)
+        self.assertEquals(res.status_int, 200)
+
+        images = res_dict['images']
+        self.assertEquals(len(images), 4)
+        self.assertEquals(int(images[0]['id']), 4)
+        self.assertEquals(int(images[1]['id']), 3)
+        self.assertEquals(int(images[2]['id']), 5)
+        self.assertEquals(int(images[3]['id']), 2)
+
+    def test_get_index_bad_sort_key(self):
+        """Ensure a 400 is returned when a bad sort_key is provided."""
+        req = webob.Request.blank('/images?sort_key=asdf')
+        res = req.get_response(self.api)
+        self.assertEqual(400, res.status_int)
+
+    def test_get_index_bad_sort_dir(self):
+        """Ensure a 400 is returned when a bad sort_dir is provided."""
+        req = webob.Request.blank('/images?sort_dir=asdf')
+        res = req.get_response(self.api)
+        self.assertEqual(400, res.status_int)
+
+    def test_get_index_sort_id_desc(self):
+        """
+        Tests that the /images registry API returns list of
+        public images sorted by id in descending order.
+        """
+        extra_fixture = {'id': 3,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'asdf',
+                         'size': 19,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        extra_fixture = {'id': 4,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'xyz',
+                         'size': 20,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        req = webob.Request.blank('/images?sort_key=id&sort_dir=desc')
+        res = req.get_response(self.api)
+        self.assertEquals(res.status_int, 200)
+        res_dict = json.loads(res.body)
+
+        images = res_dict['images']
+        self.assertEquals(len(images), 3)
+        self.assertEquals(int(images[0]['id']), 4)
+        self.assertEquals(int(images[1]['id']), 3)
+        self.assertEquals(int(images[2]['id']), 2)
+
+    def test_get_index_sort_name_asc(self):
+        """
+        Tests that the /images registry API returns list of
+        public images sorted alphabetically by name in
+        ascending order.
+        """
+        extra_fixture = {'id': 3,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'asdf',
+                         'size': 19,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        extra_fixture = {'id': 4,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'xyz',
+                         'size': 20,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        req = webob.Request.blank('/images?sort_key=name&sort_dir=asc')
+        res = req.get_response(self.api)
+        self.assertEquals(res.status_int, 200)
+        res_dict = json.loads(res.body)
+
+        images = res_dict['images']
+        self.assertEquals(len(images), 3)
+        self.assertEquals(int(images[0]['id']), 3)
+        self.assertEquals(int(images[1]['id']), 2)
+        self.assertEquals(int(images[2]['id']), 4)
+
+    def test_get_index_sort_status_desc(self):
+        """
+        Tests that the /images registry API returns list of
+        public images sorted alphabetically by status in
+        descending order.
+        """
+        extra_fixture = {'id': 3,
+                         'status': 'killed',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'asdf',
+                         'size': 19,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        extra_fixture = {'id': 4,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'xyz',
+                         'size': 20,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        req = webob.Request.blank('/images?sort_key=status&sort_dir=desc')
+        res = req.get_response(self.api)
+        self.assertEquals(res.status_int, 200)
+        res_dict = json.loads(res.body)
+
+        images = res_dict['images']
+        self.assertEquals(len(images), 3)
+        self.assertEquals(int(images[0]['id']), 3)
+        self.assertEquals(int(images[1]['id']), 4)
+        self.assertEquals(int(images[2]['id']), 2)
+
+    def test_get_index_sort_disk_format_asc(self):
+        """
+        Tests that the /images registry API returns list of
+        public images sorted alphabetically by disk_format in
+        ascending order.
+        """
+        extra_fixture = {'id': 3,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'ami',
+                         'container_format': 'ami',
+                         'name': 'asdf',
+                         'size': 19,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        extra_fixture = {'id': 4,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'vdi',
+                         'container_format': 'ovf',
+                         'name': 'xyz',
+                         'size': 20,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        req = webob.Request.blank('/images?sort_key=disk_format&sort_dir=asc')
+        res = req.get_response(self.api)
+        self.assertEquals(res.status_int, 200)
+        res_dict = json.loads(res.body)
+
+        images = res_dict['images']
+        self.assertEquals(len(images), 3)
+        self.assertEquals(int(images[0]['id']), 3)
+        self.assertEquals(int(images[1]['id']), 4)
+        self.assertEquals(int(images[2]['id']), 2)
+
+    def test_get_index_sort_container_format_desc(self):
+        """
+        Tests that the /images registry API returns list of
+        public images sorted alphabetically by container_format in
+        descending order.
+        """
+        extra_fixture = {'id': 3,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'ami',
+                         'container_format': 'ami',
+                         'name': 'asdf',
+                         'size': 19,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        extra_fixture = {'id': 4,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'iso',
+                         'container_format': 'bare',
+                         'name': 'xyz',
+                         'size': 20,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        url = '/images?sort_key=container_format&sort_dir=desc'
+        req = webob.Request.blank(url)
+        res = req.get_response(self.api)
+        self.assertEquals(res.status_int, 200)
+        res_dict = json.loads(res.body)
+
+        images = res_dict['images']
+        self.assertEquals(len(images), 3)
+        self.assertEquals(int(images[0]['id']), 2)
+        self.assertEquals(int(images[1]['id']), 4)
+        self.assertEquals(int(images[2]['id']), 3)
+
+    def test_get_index_sort_size_asc(self):
+        """
+        Tests that the /images registry API returns list of
+        public images sorted by size in ascending order.
+        """
+        extra_fixture = {'id': 3,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'ami',
+                         'container_format': 'ami',
+                         'name': 'asdf',
+                         'size': 100,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        extra_fixture = {'id': 4,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'iso',
+                         'container_format': 'bare',
+                         'name': 'xyz',
+                         'size': 2,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        url = '/images?sort_key=size&sort_dir=asc'
+        req = webob.Request.blank(url)
+        res = req.get_response(self.api)
+        self.assertEquals(res.status_int, 200)
+        res_dict = json.loads(res.body)
+
+        images = res_dict['images']
+        self.assertEquals(len(images), 3)
+        self.assertEquals(int(images[0]['id']), 4)
+        self.assertEquals(int(images[1]['id']), 2)
+        self.assertEquals(int(images[2]['id']), 3)
+
+    def test_get_index_sort_created_at_asc(self):
+        """
+        Tests that the /images registry API returns list of
+        public images sorted by created_at in ascending order.
+        """
+        now = datetime.datetime.utcnow()
+        time1 = now + datetime.timedelta(seconds=5)
+        time2 = now
+
+        extra_fixture = {'id': 3,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'new name! #123',
+                         'size': 19,
+                         'checksum': None,
+                         'created_at': time1}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        extra_fixture = {'id': 4,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'new name! #123',
+                         'size': 20,
+                         'checksum': None,
+                         'created_at': time2}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        req = webob.Request.blank('/images?sort_key=created_at&sort_dir=asc')
+        res = req.get_response(self.api)
+        self.assertEquals(res.status_int, 200)
+        res_dict = json.loads(res.body)
+
+        images = res_dict['images']
+        self.assertEquals(len(images), 3)
+        self.assertEquals(int(images[0]['id']), 2)
+        self.assertEquals(int(images[1]['id']), 4)
+        self.assertEquals(int(images[2]['id']), 3)
+
+    def test_get_index_sort_updated_at_desc(self):
+        """
+        Tests that the /images registry API returns list of
+        public images sorted by updated_at in descending order.
+        """
+        now = datetime.datetime.utcnow()
+        time1 = now + datetime.timedelta(seconds=5)
+        time2 = now
+
+        extra_fixture = {'id': 3,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'new name! #123',
+                         'size': 19,
+                         'checksum': None,
+                         'created_at': None,
+                         'created_at': time1}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        extra_fixture = {'id': 4,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'new name! #123',
+                         'size': 20,
+                         'checksum': None,
+                         'created_at': None,
+                         'updated_at': time2}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        req = webob.Request.blank('/images?sort_key=updated_at&sort_dir=desc')
+        res = req.get_response(self.api)
+        self.assertEquals(res.status_int, 200)
+        res_dict = json.loads(res.body)
+
+        images = res_dict['images']
+        self.assertEquals(len(images), 3)
+        self.assertEquals(int(images[0]['id']), 3)
+        self.assertEquals(int(images[1]['id']), 4)
+        self.assertEquals(int(images[2]['id']), 2)
+
     def test_get_details(self):
         """Tests that the /images/detail registry API returns
         a mapping containing a list of detailed image information
@@ -668,6 +1060,45 @@
         for image in images:
             self.assertEqual('v a', image['properties']['prop_123'])
 
+    def test_get_details_sort_name_asc(self):
+        """
+        Tests that the /images/details registry API returns list of
+        public images sorted alphabetically by name in
+        ascending order.
+        """
+        extra_fixture = {'id': 3,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'asdf',
+                         'size': 19,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        extra_fixture = {'id': 4,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'xyz',
+                         'size': 20,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        req = webob.Request.blank('/images/detail?sort_key=name&sort_dir=asc')
+        res = req.get_response(self.api)
+        self.assertEquals(res.status_int, 200)
+        res_dict = json.loads(res.body)
+
+        images = res_dict['images']
+        self.assertEquals(len(images), 3)
+        self.assertEquals(int(images[0]['id']), 3)
+        self.assertEquals(int(images[1]['id']), 2)
+        self.assertEquals(int(images[2]['id']), 4)
+
     def test_create_image(self):
         """Tests that the /images POST registry API creates the image"""
         fixture = {'name': 'fake public image',
@@ -1042,6 +1473,45 @@
                         "res.headerlist = %r" % res.headerlist)
         self.assertTrue('/images/3' in res.headers['location'])
 
+    def test_get_index_sort_name_asc(self):
+        """
+        Tests that the /images registry API returns list of
+        public images sorted alphabetically by name in
+        ascending order.
+        """
+        extra_fixture = {'id': 3,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'asdf',
+                         'size': 19,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        extra_fixture = {'id': 4,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'xyz',
+                         'size': 20,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        req = webob.Request.blank('/images?sort_key=name&sort_dir=asc')
+        res = req.get_response(self.api)
+        self.assertEquals(res.status_int, 200)
+        res_dict = json.loads(res.body)
+
+        images = res_dict['images']
+        self.assertEquals(len(images), 3)
+        self.assertEquals(int(images[0]['id']), 3)
+        self.assertEquals(int(images[1]['id']), 2)
+        self.assertEquals(int(images[2]['id']), 4)
+
     def test_image_is_checksummed(self):
         """Test that the image contents are checksummed properly"""
         fixture_headers = {'x-image-meta-store': 'file',

=== modified file 'tests/unit/test_clients.py'
--- tests/unit/test_clients.py	2011-05-31 19:32:17 +0000
+++ tests/unit/test_clients.py	2011-06-28 13:52:02 +0000
@@ -15,6 +15,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import datetime
 import json
 import os
 import stubout
@@ -37,7 +38,7 @@
     def test_bad_address(self):
         """Test ClientConnectionError raised"""
         c = client.Client("127.999.1.1")
-        self.assertRaises(client.ClientConnectionError,
+        self.assertRaises(exception.ClientConnectionError,
                           c.get_image,
                           1)
 
@@ -70,6 +71,298 @@
         for k, v in fixture.items():
             self.assertEquals(v, images[0][k])
 
+    def test_get_index_sort_id_desc(self):
+        """
+        Tests that the /images registry API returns list of
+        public images sorted by id in descending order.
+        """
+        extra_fixture = {'id': 3,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'asdf',
+                         'size': 19,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        extra_fixture = {'id': 4,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'xyz',
+                         'size': 20,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        images = self.client.get_images(sort_key='id', sort_dir='desc')
+
+        self.assertEquals(len(images), 3)
+        self.assertEquals(int(images[0]['id']), 4)
+        self.assertEquals(int(images[1]['id']), 3)
+        self.assertEquals(int(images[2]['id']), 2)
+
+    def test_get_index_sort_name_asc(self):
+        """
+        Tests that the /images registry API returns list of
+        public images sorted alphabetically by name in
+        ascending order.
+        """
+        extra_fixture = {'id': 3,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'asdf',
+                         'size': 19,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        extra_fixture = {'id': 4,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'xyz',
+                         'size': 20,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        images = self.client.get_images(sort_key='name', sort_dir='asc')
+
+        self.assertEquals(len(images), 3)
+        self.assertEquals(int(images[0]['id']), 3)
+        self.assertEquals(int(images[1]['id']), 2)
+        self.assertEquals(int(images[2]['id']), 4)
+
+    def test_get_index_sort_status_desc(self):
+        """
+        Tests that the /images registry API returns list of
+        public images sorted alphabetically by status in
+        descending order.
+        """
+        extra_fixture = {'id': 3,
+                         'status': 'killed',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'asdf',
+                         'size': 19,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        extra_fixture = {'id': 4,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'xyz',
+                         'size': 20,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        images = self.client.get_images(sort_key='status', sort_dir='desc')
+
+        self.assertEquals(len(images), 3)
+        self.assertEquals(int(images[0]['id']), 3)
+        self.assertEquals(int(images[1]['id']), 4)
+        self.assertEquals(int(images[2]['id']), 2)
+
+    def test_get_index_sort_disk_format_asc(self):
+        """
+        Tests that the /images registry API returns list of
+        public images sorted alphabetically by disk_format in
+        ascending order.
+        """
+        extra_fixture = {'id': 3,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'ami',
+                         'container_format': 'ami',
+                         'name': 'asdf',
+                         'size': 19,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        extra_fixture = {'id': 4,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'vdi',
+                         'container_format': 'ovf',
+                         'name': 'xyz',
+                         'size': 20,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        images = self.client.get_images(sort_key='disk_format',
+                                        sort_dir='asc')
+
+        self.assertEquals(len(images), 3)
+        self.assertEquals(int(images[0]['id']), 3)
+        self.assertEquals(int(images[1]['id']), 4)
+        self.assertEquals(int(images[2]['id']), 2)
+
+    def test_get_index_sort_container_format_desc(self):
+        """
+        Tests that the /images registry API returns list of
+        public images sorted alphabetically by container_format in
+        descending order.
+        """
+        extra_fixture = {'id': 3,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'ami',
+                         'container_format': 'ami',
+                         'name': 'asdf',
+                         'size': 19,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        extra_fixture = {'id': 4,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'iso',
+                         'container_format': 'bare',
+                         'name': 'xyz',
+                         'size': 20,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        images = self.client.get_images(sort_key='container_format',
+                                        sort_dir='desc')
+
+        self.assertEquals(len(images), 3)
+        self.assertEquals(int(images[0]['id']), 2)
+        self.assertEquals(int(images[1]['id']), 4)
+        self.assertEquals(int(images[2]['id']), 3)
+
+    def test_get_index_sort_size_asc(self):
+        """
+        Tests that the /images registry API returns list of
+        public images sorted by size in ascending order.
+        """
+        extra_fixture = {'id': 3,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'ami',
+                         'container_format': 'ami',
+                         'name': 'asdf',
+                         'size': 100,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        extra_fixture = {'id': 4,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'iso',
+                         'container_format': 'bare',
+                         'name': 'xyz',
+                         'size': 2,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        images = self.client.get_images(sort_key='size', sort_dir='asc')
+
+        self.assertEquals(len(images), 3)
+        self.assertEquals(int(images[0]['id']), 4)
+        self.assertEquals(int(images[1]['id']), 2)
+        self.assertEquals(int(images[2]['id']), 3)
+
+    def test_get_index_sort_created_at_asc(self):
+        """
+        Tests that the /images registry API returns list of
+        public images sorted by created_at in ascending order.
+        """
+        now = datetime.datetime.utcnow()
+        time1 = now + datetime.timedelta(seconds=5)
+        time2 = now
+
+        extra_fixture = {'id': 3,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'new name! #123',
+                         'size': 19,
+                         'checksum': None,
+                         'created_at': time1}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        extra_fixture = {'id': 4,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'new name! #123',
+                         'size': 20,
+                         'checksum': None,
+                         'created_at': time2}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        images = self.client.get_images(sort_key='created_at', sort_dir='asc')
+
+        self.assertEquals(len(images), 3)
+        self.assertEquals(int(images[0]['id']), 2)
+        self.assertEquals(int(images[1]['id']), 4)
+        self.assertEquals(int(images[2]['id']), 3)
+
+    def test_get_index_sort_updated_at_desc(self):
+        """
+        Tests that the /images registry API returns list of
+        public images sorted by updated_at in descending order.
+        """
+        now = datetime.datetime.utcnow()
+        time1 = now + datetime.timedelta(seconds=5)
+        time2 = now
+
+        extra_fixture = {'id': 3,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'new name! #123',
+                         'size': 19,
+                         'checksum': None,
+                         'created_at': None,
+                         'created_at': time1}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        extra_fixture = {'id': 4,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'new name! #123',
+                         'size': 20,
+                         'checksum': None,
+                         'created_at': None,
+                         'updated_at': time2}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        images = self.client.get_images(sort_key='updated_at', sort_dir='desc')
+
+        self.assertEquals(len(images), 3)
+        self.assertEquals(int(images[0]['id']), 3)
+        self.assertEquals(int(images[1]['id']), 4)
+        self.assertEquals(int(images[2]['id']), 2)
+
     def test_get_image_index_marker(self):
         """Test correct set of images returned with marker param."""
         extra_fixture = {'id': 3,
@@ -170,7 +463,7 @@
 
         glance.registry.db.api.image_create(None, extra_fixture)
 
-        images = self.client.get_images({'name': 'new name! #123'})
+        images = self.client.get_images(filters={'name': 'new name! #123'})
         self.assertEquals(len(images), 1)
 
         for image in images:
@@ -236,7 +529,8 @@
 
         glance.registry.db.api.image_create(None, extra_fixture)
 
-        images = self.client.get_images_detailed({'name': 'new name! #123'})
+        filters = {'name': 'new name! #123'}
+        images = self.client.get_images_detailed(filters=filters)
         self.assertEquals(len(images), 1)
 
         for image in images:
@@ -255,7 +549,7 @@
 
         glance.registry.db.api.image_create(None, extra_fixture)
 
-        images = self.client.get_images_detailed({'status': 'saving'})
+        images = self.client.get_images_detailed(filters={'status': 'saving'})
         self.assertEquals(len(images), 1)
 
         for image in images:
@@ -274,7 +568,8 @@
 
         glance.registry.db.api.image_create(None, extra_fixture)
 
-        images = self.client.get_images_detailed({'container_format': 'ovf'})
+        filters = {'container_format': 'ovf'}
+        images = self.client.get_images_detailed(filters=filters)
         self.assertEquals(len(images), 2)
 
         for image in images:
@@ -293,7 +588,8 @@
 
         glance.registry.db.api.image_create(None, extra_fixture)
 
-        images = self.client.get_images_detailed({'disk_format': 'vhd'})
+        filters = {'disk_format': 'vhd'}
+        images = self.client.get_images_detailed(filters=filters)
         self.assertEquals(len(images), 2)
 
         for image in images:
@@ -312,7 +608,7 @@
 
         glance.registry.db.api.image_create(None, extra_fixture)
 
-        images = self.client.get_images_detailed({'size_max': 20})
+        images = self.client.get_images_detailed(filters={'size_max': 20})
         self.assertEquals(len(images), 1)
 
         for image in images:
@@ -331,7 +627,7 @@
 
         glance.registry.db.api.image_create(None, extra_fixture)
 
-        images = self.client.get_images_detailed({'size_min': 20})
+        images = self.client.get_images_detailed(filters={'size_min': 20})
         self.assertEquals(len(images), 1)
 
         for image in images:
@@ -351,12 +647,49 @@
 
         glance.registry.db.api.image_create(None, extra_fixture)
 
-        images = self.client.get_images_detailed({'property-p a': 'v a'})
+        filters = {'property-p a': 'v a'}
+        images = self.client.get_images_detailed(filters=filters)
         self.assertEquals(len(images), 1)
 
         for image in images:
             self.assertEquals('v a', image['properties']['p a'])
 
+    def test_get_image_details_sort_disk_format_asc(self):
+        """
+        Tests that a detailed call returns list of
+        public images sorted alphabetically by disk_format in
+        ascending order.
+        """
+        extra_fixture = {'id': 3,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'ami',
+                         'container_format': 'ami',
+                         'name': 'asdf',
+                         'size': 19,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        extra_fixture = {'id': 4,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'vdi',
+                         'container_format': 'ovf',
+                         'name': 'xyz',
+                         'size': 20,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        images = self.client.get_images_detailed(sort_key='disk_format',
+                                                 sort_dir='asc')
+
+        self.assertEquals(len(images), 3)
+        self.assertEquals(int(images[0]['id']), 3)
+        self.assertEquals(int(images[1]['id']), 4)
+        self.assertEquals(int(images[2]['id']), 2)
+
     def test_get_image(self):
         """Tests that the detailed info about an image returned"""
         fixture = {'id': 1,
@@ -562,6 +895,42 @@
                           self.client.get_image,
                           3)
 
+    def test_get_image_index_sort_container_format_desc(self):
+        """
+        Tests that the client returns list of public images
+        sorted alphabetically by container_format in
+        descending order.
+        """
+        extra_fixture = {'id': 3,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'ami',
+                         'container_format': 'ami',
+                         'name': 'asdf',
+                         'size': 19,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        extra_fixture = {'id': 4,
+                         'status': 'active',
+                         'is_public': True,
+                         'disk_format': 'iso',
+                         'container_format': 'bare',
+                         'name': 'xyz',
+                         'size': 20,
+                         'checksum': None}
+
+        glance.registry.db.api.image_create(None, extra_fixture)
+
+        images = self.client.get_images(sort_key='container_format',
+                                        sort_dir='desc')
+
+        self.assertEquals(len(images), 3)
+        self.assertEquals(int(images[0]['id']), 2)
+        self.assertEquals(int(images[1]['id']), 4)
+        self.assertEquals(int(images[2]['id']), 3)
+
     def test_get_image_index(self):
         """Test correct set of public image returned"""
         fixture = {'id': 2,
@@ -671,7 +1040,8 @@
 
         glance.registry.db.api.image_create(None, extra_fixture)
 
-        images = self.client.get_images({'name': 'new name! #123'})
+        filters = {'name': 'new name! #123'}
+        images = self.client.get_images(filters=filters)
 
         self.assertEquals(len(images), 1)
         self.assertEquals('new name! #123', images[0]['name'])
@@ -690,7 +1060,8 @@
 
         glance.registry.db.api.image_create(None, extra_fixture)
 
-        images = self.client.get_images({'property-p a': 'v a'})
+        filters = {'property-p a': 'v a'}
+        images = self.client.get_images(filters=filters)
 
         self.assertEquals(len(images), 1)
         self.assertEquals(3, images[0]['id'])
@@ -765,7 +1136,8 @@
 
         glance.registry.db.api.image_create(None, extra_fixture)
 
-        images = self.client.get_images_detailed({'name': 'new name! #123'})
+        filters = {'name': 'new name! #123'}
+        images = self.client.get_images_detailed(filters=filters)
         self.assertEquals(len(images), 1)
 
         for image in images:
@@ -785,7 +1157,8 @@
 
         glance.registry.db.api.image_create(None, extra_fixture)
 
-        images = self.client.get_images_detailed({'property-p a': 'v a'})
+        filters = {'property-p a': 'v a'}
+        images = self.client.get_images_detailed(filters=filters)
         self.assertEquals(len(images), 1)
 
         for image in images: