From 19a653c984d457d311fac4588a972295320feb7b Mon Sep 17 00:00:00 2001 From: "jaypipes@gmail.com" <> Date: Thu, 14 Oct 2010 13:16:09 -0400 Subject: [PATCH 1/7] Implements the REST call for updating image metadata in the Parallax API --- glance/parallax/controllers.py | 29 +++++++++++++------- glance/parallax/db/__init__.py | 4 +++ glance/parallax/db/sqlalchemy/api.py | 6 ++--- tests/stubs.py | 28 ++++++++++++------- tests/unit/test_parallax_api.py | 40 ++++++++++++++++++++++++++++ 5 files changed, 84 insertions(+), 23 deletions(-) diff --git a/glance/parallax/controllers.py b/glance/parallax/controllers.py index c38292c864..75f5a2be6f 100644 --- a/glance/parallax/controllers.py +++ b/glance/parallax/controllers.py @@ -98,22 +98,33 @@ def create(self, req): return dict(new_image) def update(self, req, id): - """Update is not currently supported """ - raise exc.HTTPNotImplemented() + """Updates an existing image with the registry. + + :param req: Request body. A JSON-ified dict of information about + the image. This will replace the information in the + registry about this image + :param id: The opaque internal identifier for the image + + :retval Returns the updated image information as a mapping, + + """ + image_data = json.loads(req.body) + + context = None + updated_image = db.image_update(context, id, image_data) + return dict(updated_image) @staticmethod def _make_image_dict(image): - """ Create a dict represenation of an image which we can use to + """Create a dict representation of an image which we can use to serialize the image. + """ + def _fetch_attrs(d, attrs): return dict([(a, d[a]) for a in attrs]) - # attributes common to all models - base_attrs = set(['id', 'created_at', 'updated_at', 'deleted_at', - 'deleted']) - - file_attrs = base_attrs | set(['location', 'size']) + file_attrs = db.BASE_MODEL_ATTRS | set(['location', 'size']) files = [_fetch_attrs(f, file_attrs) for f in image['files']] # TODO(sirp): should this be a dict, or a list of dicts? @@ -122,7 +133,7 @@ def _fetch_attrs(d, attrs): metadata = dict((m['key'], m['value']) for m in image['metadata'] if not m['deleted']) - image_attrs = base_attrs | set(['name', 'image_type', 'status', 'is_public']) + image_attrs = db.BASE_MODEL_ATTRS | set(['name', 'image_type', 'status', 'is_public']) image_dict = _fetch_attrs(image, image_attrs) image_dict['files'] = files diff --git a/glance/parallax/db/__init__.py b/glance/parallax/db/__init__.py index 87895236b5..09fd774ef4 100644 --- a/glance/parallax/db/__init__.py +++ b/glance/parallax/db/__init__.py @@ -21,3 +21,7 @@ """ from glance.parallax.db.api import * + +# attributes common to all models +BASE_MODEL_ATTRS = set(['id', 'created_at', 'updated_at', 'deleted_at', + 'deleted']) diff --git a/glance/parallax/db/sqlalchemy/api.py b/glance/parallax/db/sqlalchemy/api.py index 9cca2c966e..e13eddf1a9 100644 --- a/glance/parallax/db/sqlalchemy/api.py +++ b/glance/parallax/db/sqlalchemy/api.py @@ -53,8 +53,7 @@ def _deleted(context): def image_create(_context, values): image_ref = models.Image() - for (key, value) in values.iteritems(): - image_ref[key] = value + image_ref.update(values) image_ref.save() return image_ref @@ -107,8 +106,7 @@ def image_update(_context, image_id, values): session = get_session() with session.begin(): image_ref = models.Image.find(image_id, session=session) - for (key, value) in values.iteritems(): - image_ref[key] = value + image_ref.update(values) image_ref.save(session=session) diff --git a/tests/stubs.py b/tests/stubs.py index b22c54f948..7dcf0d3acc 100644 --- a/tests/stubs.py +++ b/tests/stubs.py @@ -20,6 +20,7 @@ import datetime import httplib import StringIO +import sys import stubout @@ -191,7 +192,7 @@ class FakeDatastore(object): VALID_STATUSES = ('available', 'disabled', 'pending') def __init__(self): - self.images = self.FIXTURES + self.images = FakeDatastore.FIXTURES self.next_id = 3 def image_create(self, _context, values): @@ -204,22 +205,27 @@ def image_create(self, _context, values): raise exception.Invalid("Invalid status '%s' for image" % values['status']) self.next_id += 1 - self.images.extend(values) + self.images.append(values) return values + def image_update(self, _context, image_id, values): + image = self.image_get(_context, image_id) + image.update(values) + return image + def image_destroy(self, _context, image_id): - try: - del self.images[image_id] - except KeyError: - new_exc = exception.NotFound("No model for id %s" % image_id) - raise new_exc.__class__, new_exc, sys.exc_info()[2] + image = self.image_get(_context, image_id) + self.images.remove(image) def image_get(self, _context, image_id): - if image_id not in self.images.keys() or self.images[image_id]['deleted']: - new_exc = exception.NotFound("No model for id %s" % image_id) + + images = [i for i in self.images if str(i['id']) == str(image_id)] + + if len(images) != 1 or images[0]['deleted']: + new_exc = exception.NotFound("No model for id %s %s" % (image_id, str(self.images))) raise new_exc.__class__, new_exc, sys.exc_info()[2] else: - return self.images[image_id] + return images[0] def image_get_all_public(self, _context, public): return [f for f in self.images @@ -228,6 +234,8 @@ def image_get_all_public(self, _context, public): fake_datastore = FakeDatastore() stubs.Set(glance.parallax.db.sqlalchemy.api, 'image_create', fake_datastore.image_create) + stubs.Set(glance.parallax.db.sqlalchemy.api, 'image_update', + fake_datastore.image_update) stubs.Set(glance.parallax.db.sqlalchemy.api, 'image_destroy', fake_datastore.image_destroy) stubs.Set(glance.parallax.db.sqlalchemy.api, 'image_get', diff --git a/tests/unit/test_parallax_api.py b/tests/unit/test_parallax_api.py index 0046e57b7a..d004d2c7c1 100644 --- a/tests/unit/test_parallax_api.py +++ b/tests/unit/test_parallax_api.py @@ -140,3 +140,43 @@ def test_create_image_with_bad_status(self): # over to Glance to support exception catching into # standard HTTP errors. self.assertRaises(exception.Invalid, req.get_response, controllers.API()) + + def test_update_image(self): + """Tests that the /images PUT parallax API updates the image""" + fixture = {'name': 'fake public image #2', + 'image_type': 'ramdisk' + } + + req = webob.Request.blank('/images/2') + + req.method = 'PUT' + req.body = json.dumps(fixture) + + res = req.get_response(controllers.API()) + + self.assertEquals(res.status_int, 200) + + res_dict = json.loads(res.body) + + for k,v in fixture.iteritems(): + self.assertEquals(v, res_dict[k]) + + def test_update_image_not_existing(self): + """Tests proper exception is raised if attempt to update non-existing + image""" + fixture = {'id': 3, + 'name': 'fake public image', + 'is_public': True, + 'image_type': 'kernel', + 'status': 'bad status' + } + + req = webob.Request.blank('/images/3') + + req.method = 'PUT' + req.body = json.dumps(fixture) + + # TODO(jaypipes): Port Nova's Fault infrastructure + # over to Glance to support exception catching into + # standard HTTP errors. + self.assertRaises(exception.NotFound, req.get_response, controllers.API()) From 728b4e5f5f24db398e44d233b992ca0bc38fe763 Mon Sep 17 00:00:00 2001 From: "jaypipes@gmail.com" <> Date: Thu, 14 Oct 2010 13:23:37 -0400 Subject: [PATCH 2/7] Adds DELETE to the Parallax REST API. --- glance/parallax/controllers.py | 12 ++++++++-- tests/stubs.py | 6 +++-- tests/unit/test_parallax_api.py | 42 +++++++++++++++++++++++++++++++++ 3 files changed, 56 insertions(+), 4 deletions(-) diff --git a/glance/parallax/controllers.py b/glance/parallax/controllers.py index 75f5a2be6f..b4b8537776 100644 --- a/glance/parallax/controllers.py +++ b/glance/parallax/controllers.py @@ -74,8 +74,16 @@ def show(self, req, id): return dict(image=self._make_image_dict(image)) def delete(self, req, id): - """Delete is not currently supported """ - raise exc.HTTPNotImplemented() + """Deletes an existing image with the registry. + + :param req: Request body. Ignored. + :param id: The opaque internal identifier for the image + + :retval Returns 200 if delete was successful, a fault if not. + + """ + context = None + updated_image = db.image_destroy(context, id) def create(self, req): """Registers a new image with the registry. diff --git a/tests/stubs.py b/tests/stubs.py index 7dcf0d3acc..c8594f84a5 100644 --- a/tests/stubs.py +++ b/tests/stubs.py @@ -202,7 +202,8 @@ def image_create(self, _context, values): values['status'] = 'available' else: if not values['status'] in self.VALID_STATUSES: - raise exception.Invalid("Invalid status '%s' for image" % values['status']) + raise exception.Invalid("Invalid status '%s' for image" % + values['status']) self.next_id += 1 self.images.append(values) @@ -222,7 +223,8 @@ def image_get(self, _context, image_id): images = [i for i in self.images if str(i['id']) == str(image_id)] if len(images) != 1 or images[0]['deleted']: - new_exc = exception.NotFound("No model for id %s %s" % (image_id, str(self.images))) + new_exc = exception.NotFound("No model for id %s %s" % + (image_id, str(self.images))) raise new_exc.__class__, new_exc, sys.exc_info()[2] else: return images[0] diff --git a/tests/unit/test_parallax_api.py b/tests/unit/test_parallax_api.py index d004d2c7c1..33a59f9a22 100644 --- a/tests/unit/test_parallax_api.py +++ b/tests/unit/test_parallax_api.py @@ -180,3 +180,45 @@ def test_update_image_not_existing(self): # over to Glance to support exception catching into # standard HTTP errors. self.assertRaises(exception.NotFound, req.get_response, controllers.API()) + + def test_delete_image(self): + """Tests that the /images DELETE parallax API deletes the image""" + + # Grab the original number of images + req = webob.Request.blank('/images') + res = req.get_response(controllers.API()) + res_dict = json.loads(res.body) + self.assertEquals(res.status_int, 200) + + orig_num_images = len(res_dict['images']) + + # Delete image #2 + req = webob.Request.blank('/images/2') + + req.method = 'DELETE' + + res = req.get_response(controllers.API()) + + self.assertEquals(res.status_int, 200) + + # Verify one less image + req = webob.Request.blank('/images') + res = req.get_response(controllers.API()) + res_dict = json.loads(res.body) + self.assertEquals(res.status_int, 200) + + new_num_images = len(res_dict['images']) + self.assertEquals(new_num_images, orig_num_images - 1) + + def test_delete_image_not_existing(self): + """Tests proper exception is raised if attempt to delete non-existing + image""" + + req = webob.Request.blank('/images/3') + + req.method = 'DELETE' + + # TODO(jaypipes): Port Nova's Fault infrastructure + # over to Glance to support exception catching into + # standard HTTP errors. + self.assertRaises(exception.NotFound, req.get_response, controllers.API()) From da2142a7f655e4a7c03178d4d4a46248487e02b8 Mon Sep 17 00:00:00 2001 From: "jaypipes@gmail.com" <> Date: Fri, 15 Oct 2010 15:46:21 -0400 Subject: [PATCH 3/7] Ignore virtualenv directory in bzr --- .bzrignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.bzrignore b/.bzrignore index d262b2332d..f455372b51 100644 --- a/.bzrignore +++ b/.bzrignore @@ -1,3 +1,4 @@ *.pyc glance.egg-info glance.sqlite +*.glance-venv From 20749d8f3040460f0fbacf55de050df3d0c886ac Mon Sep 17 00:00:00 2001 From: "jaypipes@gmail.com" <> Date: Fri, 15 Oct 2010 15:59:52 -0400 Subject: [PATCH 4/7] Make returned mapping have an 'image' key to help in XML serialization --- glance/parallax/controllers.py | 4 ++-- tests/unit/test_parallax_api.py | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/glance/parallax/controllers.py b/glance/parallax/controllers.py index b4b8537776..c0a95474fb 100644 --- a/glance/parallax/controllers.py +++ b/glance/parallax/controllers.py @@ -103,7 +103,7 @@ def create(self, req): context = None new_image = db.image_create(context, image_data) - return dict(new_image) + return dict(image=new_image) def update(self, req, id): """Updates an existing image with the registry. @@ -120,7 +120,7 @@ def update(self, req, id): context = None updated_image = db.image_update(context, id, image_data) - return dict(updated_image) + return dict(image=updated_image) @staticmethod def _make_image_dict(image): diff --git a/tests/unit/test_parallax_api.py b/tests/unit/test_parallax_api.py index 33a59f9a22..3964fef263 100644 --- a/tests/unit/test_parallax_api.py +++ b/tests/unit/test_parallax_api.py @@ -114,13 +114,13 @@ def test_create_image(self): res_dict = json.loads(res.body) for k,v in fixture.iteritems(): - self.assertEquals(v, res_dict[k]) + self.assertEquals(v, res_dict['image'][k]) # Test ID auto-assigned properly - self.assertEquals(3, res_dict['id']) + self.assertEquals(3, res_dict['image']['id']) # Test status was updated properly - self.assertEquals('available', res_dict['status']) + self.assertEquals('available', res_dict['image']['status']) def test_create_image_with_bad_status(self): """Tests proper exception is raised if a bad status is set""" @@ -159,7 +159,7 @@ def test_update_image(self): res_dict = json.loads(res.body) for k,v in fixture.iteritems(): - self.assertEquals(v, res_dict[k]) + self.assertEquals(v, res_dict['image'][k]) def test_update_image_not_existing(self): """Tests proper exception is raised if attempt to update non-existing From 86ed4327900bf3108629038ea63c6b3c505fdde5 Mon Sep 17 00:00:00 2001 From: "jaypipes@gmail.com" <> Date: Fri, 15 Oct 2010 16:03:13 -0400 Subject: [PATCH 5/7] Quick fix...gives base Model an update() method to make it behave like a dict. --- glance/parallax/db/sqlalchemy/models.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/glance/parallax/db/sqlalchemy/models.py b/glance/parallax/db/sqlalchemy/models.py index 17ed33b7ed..a5a10a4bb5 100644 --- a/glance/parallax/db/sqlalchemy/models.py +++ b/glance/parallax/db/sqlalchemy/models.py @@ -106,6 +106,11 @@ def delete(self, session=None): self.deleted_at = datetime.datetime.utcnow() self.save(session=session) + def update(self, values): + """dict.update() behaviour.""" + for k, v in values.iteritems(): + self[k] = v + def __setitem__(self, key, value): setattr(self, key, value) From 9382efb8498b40628753698367d11ea517799a87 Mon Sep 17 00:00:00 2001 From: Rick Harris Date: Sun, 17 Oct 2010 11:31:04 -0500 Subject: [PATCH 6/7] PUTing and POSTing using image key --- glance/parallax/controllers.py | 4 ++-- tests/unit/test_parallax_api.py | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/glance/parallax/controllers.py b/glance/parallax/controllers.py index c0a95474fb..3926cb82a7 100644 --- a/glance/parallax/controllers.py +++ b/glance/parallax/controllers.py @@ -96,7 +96,7 @@ def create(self, req): in the 'id' field """ - image_data = json.loads(req.body) + image_data = json.loads(req.body)['image'] # Ensure the image has a status set image_data.setdefault('status', 'available') @@ -116,7 +116,7 @@ def update(self, req, id): :retval Returns the updated image information as a mapping, """ - image_data = json.loads(req.body) + image_data = json.loads(req.body)['image'] context = None updated_image = db.image_update(context, id, image_data) diff --git a/tests/unit/test_parallax_api.py b/tests/unit/test_parallax_api.py index 3964fef263..c9b787e596 100644 --- a/tests/unit/test_parallax_api.py +++ b/tests/unit/test_parallax_api.py @@ -105,7 +105,7 @@ def test_create_image(self): req = webob.Request.blank('/images') req.method = 'POST' - req.body = json.dumps(fixture) + req.body = json.dumps(dict(image=fixture)) res = req.get_response(controllers.API()) @@ -134,7 +134,7 @@ def test_create_image_with_bad_status(self): req = webob.Request.blank('/images') req.method = 'POST' - req.body = json.dumps(fixture) + req.body = json.dumps(dict(image=fixture)) # TODO(jaypipes): Port Nova's Fault infrastructure # over to Glance to support exception catching into @@ -150,7 +150,7 @@ def test_update_image(self): req = webob.Request.blank('/images/2') req.method = 'PUT' - req.body = json.dumps(fixture) + req.body = json.dumps(dict(image=fixture)) res = req.get_response(controllers.API()) @@ -174,7 +174,7 @@ def test_update_image_not_existing(self): req = webob.Request.blank('/images/3') req.method = 'PUT' - req.body = json.dumps(fixture) + req.body = json.dumps(dict(image=fixture)) # TODO(jaypipes): Port Nova's Fault infrastructure # over to Glance to support exception catching into From 98dc44c10e6db4dc0079883e8b88cfa9ab34febe Mon Sep 17 00:00:00 2001 From: Rick Harris Date: Sun, 17 Oct 2010 11:43:19 -0500 Subject: [PATCH 7/7] Moving ATTR helpers into db module --- glance/parallax/controllers.py | 6 ++---- glance/parallax/db/__init__.py | 5 +++++ 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/glance/parallax/controllers.py b/glance/parallax/controllers.py index 3926cb82a7..0043c17154 100644 --- a/glance/parallax/controllers.py +++ b/glance/parallax/controllers.py @@ -132,8 +132,7 @@ def _make_image_dict(image): def _fetch_attrs(d, attrs): return dict([(a, d[a]) for a in attrs]) - file_attrs = db.BASE_MODEL_ATTRS | set(['location', 'size']) - files = [_fetch_attrs(f, file_attrs) for f in image['files']] + files = [_fetch_attrs(f, db.IMAGE_FILE_ATTRS) for f in image['files']] # TODO(sirp): should this be a dict, or a list of dicts? # A plain dict is more convenient, but list of dicts would provide @@ -141,8 +140,7 @@ def _fetch_attrs(d, attrs): metadata = dict((m['key'], m['value']) for m in image['metadata'] if not m['deleted']) - image_attrs = db.BASE_MODEL_ATTRS | set(['name', 'image_type', 'status', 'is_public']) - image_dict = _fetch_attrs(image, image_attrs) + image_dict = _fetch_attrs(image, db.IMAGE_ATTRS) image_dict['files'] = files image_dict['metadata'] = metadata diff --git a/glance/parallax/db/__init__.py b/glance/parallax/db/__init__.py index 09fd774ef4..d32d0affbb 100644 --- a/glance/parallax/db/__init__.py +++ b/glance/parallax/db/__init__.py @@ -25,3 +25,8 @@ # attributes common to all models BASE_MODEL_ATTRS = set(['id', 'created_at', 'updated_at', 'deleted_at', 'deleted']) + +IMAGE_FILE_ATTRS = BASE_MODEL_ATTRS | set(['location', 'size']) + +IMAGE_ATTRS = BASE_MODEL_ATTRS | set(['name', 'image_type', 'status', + 'is_public'])