From 386b42c17631aa1f4221f1c855b0dc39e101d608 Mon Sep 17 00:00:00 2001 From: Cory Wright Date: Tue, 4 Jan 2011 17:00:37 -0500 Subject: [PATCH 1/6] More PEP8 fixes --- doc/source/conf.py | 34 +++++---- setup.py | 2 +- tests/stubs.py | 19 ++--- tests/test_data.py | 6 +- tests/unit/swiftfakehttp.py | 110 +++++++++++++-------------- tests/unit/test_api.py | 37 ++++----- tests/unit/test_clients.py | 42 +++++------ tests/unit/test_registry_api.py | 28 +++---- tests/unit/test_stores.py | 16 ++-- tools/install_venv.py | 128 +++++++++++++++++--------------- 10 files changed, 221 insertions(+), 201 deletions(-) diff --git a/doc/source/conf.py b/doc/source/conf.py index 2b1f82b2aa..4077289127 100644 --- a/doc/source/conf.py +++ b/doc/source/conf.py @@ -18,7 +18,8 @@ # Glance documentation build configuration file, created by # sphinx-quickstart on Tue May 18 13:50:15 2010. # -# This file is execfile()d with the current directory set to its containing dir. +# This file is execfile()'d with the current directory set to it's containing +# dir. # # Note that not all possible configuration values are present in this # autogenerated file. @@ -26,7 +27,8 @@ # All configuration values have a default; values that are commented out # serve to show the default. -import sys, os +import os +import sys # If extensions (or modules to document with autodoc) are in another directory, # add these directories to sys.path here. If the directory is relative to the @@ -36,19 +38,25 @@ sys.path.append([os.path.abspath('../glance'), os.path.abspath('../bin') ]) -# -- General configuration ----------------------------------------------------- +# -- General configuration --------------------------------------------------- + +# Add any Sphinx extension module names here, as strings. They can be +# extensions coming with Sphinx (named 'sphinx.ext.*') or your custom ones. +extensions = ['sphinx.ext.autodoc', + 'sphinx.ext.coverage', + 'sphinx.ext.ifconfig', + 'sphinx.ext.intersphinx', + 'sphinx.ext.pngmath', + 'sphinx.ext.todo'] -# Add any Sphinx extension module names here, as strings. They can be extensions -# coming with Sphinx (named 'sphinx.ext.*') or your custom ones. -extensions = ['sphinx.ext.autodoc', 'sphinx.ext.intersphinx', 'sphinx.ext.todo', 'sphinx.ext.coverage', 'sphinx.ext.pngmath', 'sphinx.ext.ifconfig'] todo_include_todos = True # Add any paths that contain templates here, relative to this directory. templates_path = [] if os.getenv('HUDSON_PUBLISH_DOCS'): - templates_path = ['_ga', '_templates'] + templates_path = ['_ga', '_templates'] else: - templates_path = ['_templates'] + templates_path = ['_templates'] # The suffix of source filenames. source_suffix = '.rst' @@ -89,7 +97,7 @@ release = '1.0.2' # for source files. exclude_trees = [] -# The reST default role (used for this markup: `text`) to use for all documents. +# The reST default role (for this markup: `text`) to use for all documents. #default_role = None # If true, '()' will be appended to :func: etc. cross-reference text. @@ -110,7 +118,7 @@ pygments_style = 'sphinx' modindex_common_prefix = ['glance.'] -# -- Options for HTML output --------------------------------------------------- +# -- Options for HTML output ------------------------------------------------- # The theme to use for HTML and HTML Help pages. Major themes that come with # Sphinx are currently 'default' and 'sphinxdoc'. @@ -185,7 +193,7 @@ html_static_path = ['_static'] htmlhelp_basename = 'glancedoc' -# -- Options for LaTeX output -------------------------------------------------- +# -- Options for LaTeX output ------------------------------------------------ # The paper size ('letter' or 'a4'). #latex_paper_size = 'letter' @@ -194,7 +202,8 @@ htmlhelp_basename = 'glancedoc' #latex_font_size = '10pt' # Grouping the document tree into LaTeX files. List of tuples -# (source start file, target name, title, author, documentclass [howto/manual]). +# (source start file, target name, title, author, +# documentclass [howto/manual]). latex_documents = [ ('index', 'Glance.tex', u'Glance Documentation', u'Glance Team', 'manual'), @@ -221,4 +230,3 @@ latex_documents = [ intersphinx_mapping = {'python': ('http://docs.python.org/', None), 'nova': ('http://nova.openstack.org', None), 'swift': ('http://swift.openstack.org', None)} - diff --git a/setup.py b/setup.py index 5edc2c6562..2c32b7399b 100644 --- a/setup.py +++ b/setup.py @@ -56,6 +56,6 @@ setup( 'Programming Language :: Python :: 2.6', 'Environment :: No Input/Output (Daemon)', ], - install_requires=[], # removed for better compat + install_requires=[], # removed for better compat scripts=['bin/glance-api', 'bin/glance-registry']) diff --git a/tests/stubs.py b/tests/stubs.py index 02ee3b89bb..155c2c77bc 100644 --- a/tests/stubs.py +++ b/tests/stubs.py @@ -44,7 +44,7 @@ def stub_out_http_backend(stubs): """Stubs out the httplib.HTTPRequest.getresponse to return faked-out data instead of grabbing actual contents of a resource - The stubbed getresponse() returns an iterator over + The stubbed getresponse() returns an iterator over the data "I am a teapot, short and stout\n" :param stubs: Set of stubout stubs @@ -117,6 +117,7 @@ def stub_out_swift_backend(stubs): """ class FakeSwiftAuth(object): pass + class FakeSwiftConnection(object): pass @@ -135,8 +136,8 @@ def stub_out_swift_backend(stubs): def chunk_it(): for i in xrange(0, len(cls.DATA), cls.CHUNK_SIZE): - yield cls.DATA[i:i+cls.CHUNK_SIZE] - + yield cls.DATA[i:i + cls.CHUNK_SIZE] + return chunk_it() fake_swift_backend = FakeSwiftBackend() @@ -324,25 +325,25 @@ def stub_out_registry_db_image_api(stubs): values['deleted'] = False values['properties'] = values.get('properties', {}) - values['created_at'] = datetime.datetime.utcnow() + values['created_at'] = datetime.datetime.utcnow() values['updated_at'] = datetime.datetime.utcnow() values['deleted_at'] = None props = [] if 'properties' in values.keys(): - for k,v in values['properties'].iteritems(): + for k, v in values['properties'].iteritems(): p = {} p['key'] = k p['value'] = v p['deleted'] = False - p['created_at'] = datetime.datetime.utcnow() + p['created_at'] = datetime.datetime.utcnow() p['updated_at'] = datetime.datetime.utcnow() p['deleted_at'] = None props.append(p) values['properties'] = props - + self.next_id += 1 self.images.append(values) return values @@ -352,12 +353,12 @@ def stub_out_registry_db_image_api(stubs): props = [] if 'properties' in values.keys(): - for k,v in values['properties'].iteritems(): + for k, v in values['properties'].iteritems(): p = {} p['key'] = k p['value'] = v p['deleted'] = False - p['created_at'] = datetime.datetime.utcnow() + p['created_at'] = datetime.datetime.utcnow() p['updated_at'] = datetime.datetime.utcnow() p['deleted_at'] = None props.append(p) diff --git a/tests/test_data.py b/tests/test_data.py index d23a0ed814..221480be4c 100644 --- a/tests/test_data.py +++ b/tests/test_data.py @@ -23,7 +23,7 @@ def make_swift_image(): # TODO(sirp): Create a testing account, and define gflags for # test_swift_username and test_swift_api_key - USERNAME = "your user name here" # fill these out for testing + USERNAME = "your user name here" # fill these out for testing API_KEY = "your api key here" #IMAGE_CHUNKS = [("filename", 123)] # filename, size in bytes IMAGE_CHUNKS = [("your test chunk here", 12345)] @@ -40,8 +40,8 @@ def make_swift_image(): "swift://%s:%s@auth.api.rackspacecloud.com/v1.0/cloudservers/%s" ) % (USERNAME, API_KEY, obj) - db.image_file_create(None, + db.image_file_create(None, dict(image_id=image.id, location=location, size=size)) if __name__ == "__main__": - make_swift_image() # NOTE: uncomment if you have a username and api_key + make_swift_image() # NOTE: uncomment if you have a username and api_key diff --git a/tests/unit/swiftfakehttp.py b/tests/unit/swiftfakehttp.py index cbf7f7acc1..995ea8d7cc 100644 --- a/tests/unit/swiftfakehttp.py +++ b/tests/unit/swiftfakehttp.py @@ -19,13 +19,13 @@ fakehttp/socket implementation - TrackerSocket: an object which masquerades as a socket and responds to - requests in a manner consistent with a *very* stupid CloudFS tracker. - + requests in a manner consistent with a *very* stupid CloudFS tracker. + - CustomHTTPConnection: an object which subclasses httplib.HTTPConnection in order to replace it's socket with a TrackerSocket instance. -The unittests each have setup methods which create freerange connection -instances that have had their HTTPConnection instances replaced by +The unittests each have setup methods which create freerange connection +instances that have had their HTTPConnection instances replaced by intances of CustomHTTPConnection. """ @@ -54,21 +54,23 @@ class FakeSocket(object): def makefile(self, mode, flags): return self._wbuffer + class TrackerSocket(FakeSocket): def write(self, data): self._wbuffer.write(data) + def read(self, length=-1): return self._rbuffer.read(length) def _create_GET_account_content(self, path, args): - if args.has_key('format') and args['format'] == 'json': + if 'format' in args and args['format'] == 'json': containers = [] - containers.append('[\n'); + containers.append('[\n') containers.append('{"name":"container1","count":2,"bytes":78},\n') containers.append('{"name":"container2","count":1,"bytes":39},\n') containers.append('{"name":"container3","count":3,"bytes":117}\n') containers.append(']\n') - elif args.has_key('format') and args['format'] == 'xml': + elif 'format' in args and args['format'] == 'xml': containers = [] containers.append('\n') containers.append('\n') @@ -83,18 +85,18 @@ class TrackerSocket(FakeSocket): '117\n') containers.append('\n') else: - containers = ['container%s\n' % i for i in range(1,4)] + containers = ['container%s\n' % i for i in range(1, 4)] return ''.join(containers) def _create_GET_container_content(self, path, args): left = 0 right = 9 - if args.has_key('offset'): + if 'offset' in args: left = int(args['offset']) - if args.has_key('limit'): + if 'limit' in args: right = left + int(args['limit']) - if args.has_key('format') and args['format'] == 'json': + if 'format' in args and args['format'] == 'json': objects = [] objects.append('{"name":"object1",' '"hash":"4281c348eaf83e70ddce0e07221c3d28",' @@ -137,68 +139,68 @@ class TrackerSocket(FakeSocket): '"content_type":"application\/octet-stream",' '"last_modified":"2007-03-04 20:32:17"}') output = '[\n%s\n]\n' % (',\n'.join(objects[left:right])) - elif args.has_key('format') and args['format'] == 'xml': + elif 'format' in args and args['format'] == 'xml': objects = [] objects.append('object1' - '4281c348eaf83e70ddce0e07221c3d28' - '14' - 'application/octet-stream' - '2007-03-04 20:32:17' - '\n') + '4281c348eaf83e70ddce0e07221c3d28' + '14' + 'application/octet-stream' + '2007-03-04 20:32:17' + '\n') objects.append('object2' - 'b039efe731ad111bc1b0ef221c3849d0' - '64' - 'application/octet-stream' - '2007-03-04 20:32:17' - '\n') + 'b039efe731ad111bc1b0ef221c3849d0' + '64' + 'application/octet-stream' + '2007-03-04 20:32:17' + '\n') objects.append('object3' - '4281c348eaf83e70ddce0e07221c3d28' - '14' - 'application/octet-stream' - '2007-03-04 20:32:17' - '\n') + '4281c348eaf83e70ddce0e07221c3d28' + '14' + 'application/octet-stream' + '2007-03-04 20:32:17' + '\n') objects.append('object4' - 'b039efe731ad111bc1b0ef221c3849d0' - '64' - 'application/octet-stream' - '2007-03-04 20:32:17' - '\n') + 'b039efe731ad111bc1b0ef221c3849d0' + '64' + 'application/octet-stream' + '2007-03-04 20:32:17' + '\n') objects.append('object5' - '4281c348eaf83e70ddce0e07221c3d28' - '14' - 'application/octet-stream' - '2007-03-04 20:32:17' - '\n') + '4281c348eaf83e70ddce0e07221c3d28' + '14' + 'application/octet-stream' + '2007-03-04 20:32:17' + '\n') objects.append('object6' - 'b039efe731ad111bc1b0ef221c3849d0' - '64' - 'application/octet-stream' - '2007-03-04 20:32:17' - '\n') + 'b039efe731ad111bc1b0ef221c3849d0' + '64' + 'application/octet-stream' + '2007-03-04 20:32:17' + '\n') objects.append('object7' - '4281c348eaf83e70ddce0e07221c3d28' - '14' - 'application/octet-stream' - '2007-03-04 20:32:17' - '\n') + '4281c348eaf83e70ddce0e07221c3d28' + '14' + 'application/octet-stream' + '2007-03-04 20:32:17' + '\n') objects.append('object8' - 'b039efe731ad111bc1b0ef221c3849d0' - '64' - 'application/octet-stream' - '2007-03-04 20:32:17' - '\n') + 'b039efe731ad111bc1b0ef221c3849d0' + '64' + 'application/octet-stream' + '2007-03-04 20:32:17' + '\n') objects = objects[left:right] objects.insert(0, '\n') objects.insert(1, '\n') output = ''.join(objects) else: - objects = ['object%s\n' % i for i in range(1,9)] + objects = ['object%s\n' % i for i in range(1, 9)] objects = objects[left:right] output = ''.join(objects) # prefix/path don't make much sense given our test data - if args.has_key('prefix') or args.has_key('path'): + if 'prefix' in args or 'path' in args: pass return output diff --git a/tests/unit/test_api.py b/tests/unit/test_api.py index 34f83980d9..7cd8f48a64 100644 --- a/tests/unit/test_api.py +++ b/tests/unit/test_api.py @@ -45,7 +45,7 @@ class TestRegistryAPI(unittest.TestCase): def test_get_root(self): """Tests that the root registry API returns "index", which is a list of public images - + """ fixture = {'id': 2, 'name': 'fake image #2'} @@ -57,13 +57,13 @@ class TestRegistryAPI(unittest.TestCase): images = res_dict['images'] self.assertEquals(len(images), 1) - for k,v in fixture.iteritems(): + for k, v in fixture.iteritems(): self.assertEquals(v, images[0][k]) def test_get_index(self): """Tests that the /images registry API returns list of public images - + """ fixture = {'id': 2, 'name': 'fake image #2'} @@ -75,13 +75,13 @@ class TestRegistryAPI(unittest.TestCase): images = res_dict['images'] self.assertEquals(len(images), 1) - for k,v in fixture.iteritems(): + for k, v in fixture.iteritems(): self.assertEquals(v, images[0][k]) def test_get_details(self): """Tests that the /images/detail registry API returns a mapping containing a list of detailed image information - + """ fixture = {'id': 2, 'name': 'fake image #2', @@ -97,7 +97,7 @@ class TestRegistryAPI(unittest.TestCase): images = res_dict['images'] self.assertEquals(len(images), 1) - for k,v in fixture.iteritems(): + for k, v in fixture.iteritems(): self.assertEquals(v, images[0][k]) def test_create_image(self): @@ -108,7 +108,7 @@ class TestRegistryAPI(unittest.TestCase): } req = webob.Request.blank('/images') - + req.method = 'POST' req.body = json.dumps(dict(image=fixture)) @@ -118,7 +118,7 @@ class TestRegistryAPI(unittest.TestCase): res_dict = json.loads(res.body) - for k,v in fixture.iteritems(): + for k, v in fixture.iteritems(): self.assertEquals(v, res_dict['image'][k]) # Test ID auto-assigned properly @@ -137,7 +137,7 @@ class TestRegistryAPI(unittest.TestCase): } req = webob.Request.blank('/images') - + req.method = 'POST' req.body = json.dumps(dict(image=fixture)) @@ -154,7 +154,7 @@ class TestRegistryAPI(unittest.TestCase): } req = webob.Request.blank('/images/2') - + req.method = 'PUT' req.body = json.dumps(dict(image=fixture)) @@ -164,7 +164,7 @@ class TestRegistryAPI(unittest.TestCase): res_dict = json.loads(res.body) - for k,v in fixture.iteritems(): + for k, v in fixture.iteritems(): self.assertEquals(v, res_dict['image'][k]) def test_update_image_not_existing(self): @@ -178,7 +178,7 @@ class TestRegistryAPI(unittest.TestCase): } req = webob.Request.blank('/images/3') - + req.method = 'PUT' req.body = json.dumps(dict(image=fixture)) @@ -202,7 +202,7 @@ class TestRegistryAPI(unittest.TestCase): # Delete image #2 req = webob.Request.blank('/images/2') - + req.method = 'DELETE' res = req.get_response(rserver.API()) @@ -223,7 +223,7 @@ class TestRegistryAPI(unittest.TestCase): image""" req = webob.Request.blank('/images/3') - + req.method = 'DELETE' # TODO(jaypipes): Port Nova's Fault infrastructure @@ -257,7 +257,7 @@ class TestGlanceAPI(unittest.TestCase): req = webob.Request.blank("/images") req.method = 'POST' - for k,v in fixture_headers.iteritems(): + for k, v in fixture_headers.iteritems(): req.headers[k] = v res = req.get_response(server.API()) self.assertEquals(res.status_int, webob.exc.HTTPBadRequest.code) @@ -269,7 +269,7 @@ class TestGlanceAPI(unittest.TestCase): req = webob.Request.blank("/images") req.method = 'POST' - for k,v in fixture_headers.iteritems(): + for k, v in fixture_headers.iteritems(): req.headers[k] = v req.headers['Content-Type'] = 'application/octet-stream' @@ -284,7 +284,7 @@ class TestGlanceAPI(unittest.TestCase): req = webob.Request.blank("/images") req.method = 'POST' - for k,v in fixture_headers.iteritems(): + for k, v in fixture_headers.iteritems(): req.headers[k] = v req.headers['Content-Type'] = 'application/octet-stream' @@ -327,7 +327,8 @@ class TestGlanceAPI(unittest.TestCase): req = webob.Request.blank("/images/2") req.method = 'GET' res = req.get_response(server.API()) - self.assertEquals(res.status_int, webob.exc.HTTPNotFound.code, res.body) + self.assertEquals(res.status_int, webob.exc.HTTPNotFound.code, + res.body) def test_delete_non_exists_image(self): req = webob.Request.blank("/images/42") diff --git a/tests/unit/test_clients.py b/tests/unit/test_clients.py index 610ff7aaf8..7f2bbfce97 100644 --- a/tests/unit/test_clients.py +++ b/tests/unit/test_clients.py @@ -33,7 +33,7 @@ FLAGS = flags.FLAGS class TestBadClients(unittest.TestCase): - + """Test exceptions raised for bad clients""" def test_bad_address(self): @@ -69,7 +69,7 @@ class TestRegistryClient(unittest.TestCase): images = self.client.get_images() self.assertEquals(len(images), 1) - for k,v in fixture.iteritems(): + for k, v in fixture.iteritems(): self.assertEquals(v, images[0][k]) def test_get_image_details(self): @@ -95,7 +95,7 @@ class TestRegistryClient(unittest.TestCase): images = self.client.get_images_detailed() self.assertEquals(len(images), 1) - for k,v in expected.iteritems(): + for k, v in expected.iteritems(): self.assertEquals(v, images[0][k]) def test_get_image(self): @@ -120,7 +120,7 @@ class TestRegistryClient(unittest.TestCase): data = self.client.get_image(2) - for k,v in expected.iteritems(): + for k, v in expected.iteritems(): self.assertEquals(v, data[k]) def test_get_image_non_existing(self): @@ -138,7 +138,7 @@ class TestRegistryClient(unittest.TestCase): 'size': 19, 'location': "file:///tmp/glance-tests/acct/3.gz.0", } - + new_image = self.client.add_image(fixture) # Test ID auto-assigned properly @@ -147,7 +147,7 @@ class TestRegistryClient(unittest.TestCase): # Test all other attributes set data = self.client.get_image(3) - for k,v in fixture.iteritems(): + for k, v in fixture.iteritems(): self.assertEquals(v, data[k]) # Test status was updated properly @@ -170,13 +170,13 @@ class TestRegistryClient(unittest.TestCase): 'location': "file:///tmp/glance-tests/2", 'properties': {'distro': 'Ubuntu 10.04 LTS'} } - + new_image = self.client.add_image(fixture) # Test ID auto-assigned properly self.assertEquals(3, new_image['id']) - for k,v in expected.iteritems(): + for k, v in expected.iteritems(): self.assertEquals(v, new_image[k]) # Test status was updated properly @@ -224,7 +224,7 @@ class TestRegistryClient(unittest.TestCase): # Test all other attributes set data = self.client.get_image(2) - for k,v in fixture.iteritems(): + for k, v in fixture.iteritems(): self.assertEquals(v, data[k]) def test_update_image_not_existing(self): @@ -304,7 +304,7 @@ class TestClient(unittest.TestCase): image_data += image_chunk self.assertEquals(expected_image, image_data) - for k,v in expected_meta.iteritems(): + for k, v in expected_meta.iteritems(): self.assertEquals(v, meta[k]) def test_get_image_not_existing(self): @@ -321,7 +321,7 @@ class TestClient(unittest.TestCase): images = self.client.get_images() self.assertEquals(len(images), 1) - for k,v in fixture.iteritems(): + for k, v in fixture.iteritems(): self.assertEquals(v, images[0][k]) def test_get_image_details(self): @@ -347,7 +347,7 @@ class TestClient(unittest.TestCase): images = self.client.get_images_detailed() self.assertEquals(len(images), 1) - for k,v in expected.iteritems(): + for k, v in expected.iteritems(): self.assertEquals(v, images[0][k]) def test_get_image_meta(self): @@ -372,7 +372,7 @@ class TestClient(unittest.TestCase): data = self.client.get_image_meta(2) - for k,v in expected.iteritems(): + for k, v in expected.iteritems(): self.assertEquals(v, data[k]) def test_get_image_non_existing(self): @@ -388,7 +388,7 @@ class TestClient(unittest.TestCase): 'is_public': True, 'type': 'kernel' } - + self.assertRaises(exception.Invalid, self.client.add_image, fixture) @@ -401,7 +401,7 @@ class TestClient(unittest.TestCase): 'size': 19, 'location': "file:///tmp/glance-tests/2", } - + new_id = self.client.add_image(fixture) # Test ID auto-assigned properly @@ -410,7 +410,7 @@ class TestClient(unittest.TestCase): # Test all other attributes set data = self.client.get_image_meta(3) - for k,v in fixture.iteritems(): + for k, v in fixture.iteritems(): self.assertEquals(v, data[k]) # Test status was updated properly @@ -433,7 +433,7 @@ class TestClient(unittest.TestCase): 'location': "file:///tmp/glance-tests/2", 'properties': {'distro': 'Ubuntu 10.04 LTS'} } - + new_id = self.client.add_image(fixture) # Test ID auto-assigned properly @@ -442,7 +442,7 @@ class TestClient(unittest.TestCase): # Test all other attributes set data = self.client.get_image_meta(3) - for k,v in expected.iteritems(): + for k, v in expected.iteritems(): self.assertEquals(v, data[k]) # Test status was updated properly @@ -502,7 +502,7 @@ class TestClient(unittest.TestCase): new_image_data += image_chunk self.assertEquals(image_data_fixture, new_image_data) - for k,v in fixture.iteritems(): + for k, v in fixture.iteritems(): self.assertEquals(v, new_meta[k]) def test_add_image_with_image_data_as_file(self): @@ -539,7 +539,7 @@ class TestClient(unittest.TestCase): new_image_data += image_chunk self.assertEquals(image_data_fixture, new_image_data) - for k,v in fixture.iteritems(): + for k, v in fixture.iteritems(): self.assertEquals(v, new_meta[k]) def test_add_image_with_bad_store(self): @@ -570,7 +570,7 @@ class TestClient(unittest.TestCase): # Test all other attributes set data = self.client.get_image_meta(2) - for k,v in fixture.iteritems(): + for k, v in fixture.iteritems(): self.assertEquals(v, data[k]) def test_update_image_not_existing(self): diff --git a/tests/unit/test_registry_api.py b/tests/unit/test_registry_api.py index 57f2c6e50e..f32bc782f1 100644 --- a/tests/unit/test_registry_api.py +++ b/tests/unit/test_registry_api.py @@ -38,7 +38,7 @@ class TestImageController(unittest.TestCase): def test_get_root(self): """Tests that the root registry API returns "index", which is a list of public images - + """ fixture = {'id': 2, 'name': 'fake image #2'} @@ -50,13 +50,13 @@ class TestImageController(unittest.TestCase): images = res_dict['images'] self.assertEquals(len(images), 1) - for k,v in fixture.iteritems(): + for k, v in fixture.iteritems(): self.assertEquals(v, images[0][k]) def test_get_index(self): """Tests that the /images registry API returns list of public images - + """ fixture = {'id': 2, 'name': 'fake image #2'} @@ -68,13 +68,13 @@ class TestImageController(unittest.TestCase): images = res_dict['images'] self.assertEquals(len(images), 1) - for k,v in fixture.iteritems(): + for k, v in fixture.iteritems(): self.assertEquals(v, images[0][k]) def test_get_details(self): """Tests that the /images/detail registry API returns a mapping containing a list of detailed image information - + """ fixture = {'id': 2, 'name': 'fake image #2', @@ -90,7 +90,7 @@ class TestImageController(unittest.TestCase): images = res_dict['images'] self.assertEquals(len(images), 1) - for k,v in fixture.iteritems(): + for k, v in fixture.iteritems(): self.assertEquals(v, images[0][k]) def test_create_image(self): @@ -101,7 +101,7 @@ class TestImageController(unittest.TestCase): } req = webob.Request.blank('/images') - + req.method = 'POST' req.body = json.dumps(dict(image=fixture)) @@ -111,7 +111,7 @@ class TestImageController(unittest.TestCase): res_dict = json.loads(res.body) - for k,v in fixture.iteritems(): + for k, v in fixture.iteritems(): self.assertEquals(v, res_dict['image'][k]) # Test ID auto-assigned properly @@ -130,7 +130,7 @@ class TestImageController(unittest.TestCase): } req = webob.Request.blank('/images') - + req.method = 'POST' req.body = json.dumps(dict(image=fixture)) @@ -147,7 +147,7 @@ class TestImageController(unittest.TestCase): } req = webob.Request.blank('/images/2') - + req.method = 'PUT' req.body = json.dumps(dict(image=fixture)) @@ -157,7 +157,7 @@ class TestImageController(unittest.TestCase): res_dict = json.loads(res.body) - for k,v in fixture.iteritems(): + for k, v in fixture.iteritems(): self.assertEquals(v, res_dict['image'][k]) def test_update_image_not_existing(self): @@ -171,7 +171,7 @@ class TestImageController(unittest.TestCase): } req = webob.Request.blank('/images/3') - + req.method = 'PUT' req.body = json.dumps(dict(image=fixture)) @@ -195,7 +195,7 @@ class TestImageController(unittest.TestCase): # Delete image #2 req = webob.Request.blank('/images/2') - + req.method = 'DELETE' res = req.get_response(server.API()) @@ -216,7 +216,7 @@ class TestImageController(unittest.TestCase): image""" req = webob.Request.blank('/images/3') - + req.method = 'DELETE' # TODO(jaypipes): Port Nova's Fault infrastructure diff --git a/tests/unit/test_stores.py b/tests/unit/test_stores.py index adb9e491be..212a39c816 100644 --- a/tests/unit/test_stores.py +++ b/tests/unit/test_stores.py @@ -27,7 +27,9 @@ from tests import stubs Backend.CHUNKSIZE = 2 + class TestBackend(unittest.TestCase): + def setUp(self): """Establish a clean test environment""" self.stubs = stubout.StubOutForTesting() @@ -66,7 +68,7 @@ class TestHTTPBackend(TestBackend): def test_http_get(self): url = "http://netloc/path/to/file.tar.gz" - expected_returns = ['I ', 'am', ' a', ' t', 'ea', 'po', 't,', ' s', + expected_returns = ['I ', 'am', ' a', ' t', 'ea', 'po', 't,', ' s', 'ho', 'rt', ' a', 'nd', ' s', 'to', 'ut', '\n'] fetcher = get_from_backend(url, expected_size=8) @@ -76,7 +78,7 @@ class TestHTTPBackend(TestBackend): def test_https_get(self): url = "https://netloc/path/to/file.tar.gz" - expected_returns = ['I ', 'am', ' a', ' t', 'ea', 'po', 't,', ' s', + expected_returns = ['I ', 'am', ' a', ' t', 'ea', 'po', 't,', ' s', 'ho', 'rt', ' a', 'nd', ' s', 'to', 'ut', '\n'] fetcher = get_from_backend(url, expected_size=8) @@ -93,8 +95,8 @@ class TestSwiftBackend(TestBackend): def test_get(self): - swift_uri = "swift://user:password@localhost/container1/file.tar.gz" - expected_returns = ['I ', 'am', ' a', ' t', 'ea', 'po', 't,', ' s', + swift_uri = "swift://user:pass@localhost/container1/file.tar.gz" + expected_returns = ['I ', 'am', ' a', ' t', 'ea', 'po', 't,', ' s', 'ho', 'rt', ' a', 'nd', ' s', 'to', 'ut', '\n'] fetcher = get_from_backend(swift_uri, @@ -109,12 +111,12 @@ class TestSwiftBackend(TestBackend): swift_url = "swift://localhost/container1/file.tar.gz" - self.assertRaises(BackendException, get_from_backend, + self.assertRaises(BackendException, get_from_backend, swift_url, expected_size=21) def test_url_parsing(self): - swift_uri = "swift://user:password@localhost/v1.0/container1/file.tar.gz" + swift_uri = "swift://user:pass@localhost/v1.0/container1/file.tar.gz" parsed_uri = urlparse.urlparse(swift_uri) @@ -122,7 +124,7 @@ class TestSwiftBackend(TestBackend): SwiftBackend._parse_swift_tokens(parsed_uri) self.assertEqual(user, 'user') - self.assertEqual(key, 'password') + self.assertEqual(key, 'pass') self.assertEqual(authurl, 'https://localhost/v1.0') self.assertEqual(container, 'container1') self.assertEqual(obj, 'file.tar.gz') diff --git a/tools/install_venv.py b/tools/install_venv.py index 2173bb9d92..870982496b 100644 --- a/tools/install_venv.py +++ b/tools/install_venv.py @@ -30,82 +30,88 @@ import sys ROOT = os.path.dirname(os.path.dirname(os.path.realpath(__file__))) VENV = os.path.join(ROOT, '.glance-venv') PIP_REQUIRES = os.path.join(ROOT, 'tools', 'pip-requires') -TWISTED_NOVA='http://nova.openstack.org/Twisted-10.0.0Nova.tar.gz' +TWISTED_NOVA = 'http://nova.openstack.org/Twisted-10.0.0Nova.tar.gz' + def die(message, *args): - print >>sys.stderr, message % args - sys.exit(1) + print >>sys.stderr, message % args + sys.exit(1) def run_command(cmd, redirect_output=True, check_exit_code=True): - """ - Runs a command in an out-of-process shell, returning the - output of that command. Working directory is ROOT. - """ - if redirect_output: - stdout = subprocess.PIPE - else: - stdout = None + """ + Runs a command in an out-of-process shell, returning the + output of that command. Working directory is ROOT. + """ + if redirect_output: + stdout = subprocess.PIPE + else: + stdout = None - proc = subprocess.Popen(cmd, cwd=ROOT, stdout=stdout) - output = proc.communicate()[0] - if check_exit_code and proc.returncode != 0: - die('Command "%s" failed.\n%s', ' '.join(cmd), output) - return output + proc = subprocess.Popen(cmd, cwd=ROOT, stdout=stdout) + output = proc.communicate()[0] + if check_exit_code and proc.returncode != 0: + die('Command "%s" failed.\n%s', ' '.join(cmd), output) + return output -HAS_EASY_INSTALL = bool(run_command(['which', 'easy_install'], check_exit_code=False).strip()) -HAS_VIRTUALENV = bool(run_command(['which', 'virtualenv'], check_exit_code=False).strip()) +HAS_EASY_INSTALL = bool(run_command(['which', 'easy_install'], + check_exit_code=False).strip()) +HAS_VIRTUALENV = bool(run_command(['which', 'virtualenv'], + check_exit_code=False).strip()) def check_dependencies(): - """Make sure virtualenv is in the path.""" + """Make sure virtualenv is in the path.""" - if not HAS_VIRTUALENV: - print 'not found.' - # Try installing it via easy_install... - if HAS_EASY_INSTALL: - print 'Installing virtualenv via easy_install...', - if not run_command(['which', 'easy_install']): - die('ERROR: virtualenv not found.\n\nGlance development requires virtualenv,' - ' please install it using your favorite package management tool') - print 'done.' - print 'done.' + if not HAS_VIRTUALENV: + print 'not found.' + # Try installing it via easy_install... + if HAS_EASY_INSTALL: + print 'Installing virtualenv via easy_install...', + if not run_command(['which', 'easy_install']): + die('ERROR: virtualenv not found.\n\n' + 'Glance development requires virtualenv, please install' + ' it using your favorite package management tool') + print 'done.' + print 'done.' def create_virtualenv(venv=VENV): - """Creates the virtual environment and installs PIP only into the - virtual environment - """ - print 'Creating venv...', - run_command(['virtualenv', '-q', '--no-site-packages', VENV]) - print 'done.' - print 'Installing pip in virtualenv...', - if not run_command(['tools/with_venv.sh', 'easy_install', 'pip']).strip(): - die("Failed to install pip.") - print 'done.' + """Creates the virtual environment and installs PIP only into the + virtual environment + """ + print 'Creating venv...', + run_command(['virtualenv', '-q', '--no-site-packages', VENV]) + print 'done.' + print 'Installing pip in virtualenv...', + if not run_command(['tools/with_venv.sh', 'easy_install', 'pip']).strip(): + die("Failed to install pip.") + print 'done.' def install_dependencies(venv=VENV): - print 'Installing dependencies with pip (this can take a while)...' + print 'Installing dependencies with pip (this can take a while)...' - # Install greenlet by hand - just listing it in the requires file does not - # get it in stalled in the right order - run_command(['tools/with_venv.sh', 'pip', 'install', '-E', venv, 'greenlet'], - redirect_output=False) - run_command(['tools/with_venv.sh', 'pip', 'install', '-E', venv, '-r', PIP_REQUIRES], - redirect_output=False) - run_command(['tools/with_venv.sh', 'pip', 'install', '-E', venv, TWISTED_NOVA], - redirect_output=False) + # Install greenlet by hand - just listing it in the requires file does not + # get it in stalled in the right order + venv_tool = 'tools/with_venv.sh' + run_command([venv_tool, 'pip', 'install', '-E', venv, 'greenlet'], + redirect_output=False) + run_command([venv_tool, 'pip', 'install', '-E', venv, '-r', PIP_REQUIRES], + redirect_output=False) + run_command([venv_tool, 'pip', 'install', '-E', venv, TWISTED_NOVA], + redirect_output=False) - # Tell the virtual env how to "import glance" - pthfile = os.path.join(venv, "lib", "python2.6", "site-packages", "glance.pth") - f = open(pthfile, 'w') - f.write("%s\n" % ROOT) + # Tell the virtual env how to "import glance" + pthfile = os.path.join(venv, "lib", "python2.6", "site-packages", + "glance.pth") + f = open(pthfile, 'w') + f.write("%s\n" % ROOT) def print_help(): - help = """ + help = """ Glance development environment setup is complete. Glance development uses virtualenv to track and manage Python dependencies @@ -114,7 +120,7 @@ def print_help(): To activate the Glance virtualenv for the extent of your current shell session you can run: - $ source .glance-venv/bin/activate + $ source .glance-venv/bin/activate Or, if you prefer, you can run commands in the virtualenv on a case by case basis by running: @@ -122,15 +128,15 @@ def print_help(): $ tools/with_venv.sh Also, make test will automatically use the virtualenv. - """ - print help + """ + print help def main(argv): - check_dependencies() - create_virtualenv() - install_dependencies() - print_help() + check_dependencies() + create_virtualenv() + install_dependencies() + print_help() if __name__ == '__main__': - main(sys.argv) + main(sys.argv) From b09e9c9600a1581bb2aba355714d258696573c3c Mon Sep 17 00:00:00 2001 From: Rick Harris Date: Tue, 4 Jan 2011 18:06:31 -0600 Subject: [PATCH 2/6] refactoring so update can take image_data --- glance/client.py | 30 ++++----- glance/registry/db/sqlalchemy/models.py | 2 +- glance/registry/server.py | 2 +- glance/server.py | 81 +++++++++++++++++++++++++ tests/stubs.py | 8 +-- tests/unit/test_api.py | 12 ++-- tests/unit/test_clients.py | 66 ++++++++++---------- tests/unit/test_registry_api.py | 4 +- 8 files changed, 144 insertions(+), 61 deletions(-) diff --git a/glance/client.py b/glance/client.py index 6f11664dd1..1eda1c384f 100644 --- a/glance/client.py +++ b/glance/client.py @@ -120,11 +120,11 @@ class BaseClient(object): elif status_code == httplib.NOT_FOUND: raise exception.NotFound elif status_code == httplib.CONFLICT: - raise exception.Duplicate + raise exception.Duplicate(res) elif status_code == httplib.BAD_REQUEST: - raise exception.BadInputError + raise exception.BadInputError(res) else: - raise Exception("Unknown error occurred! %d" % status_code) + raise Exception("Unknown error occurred! %s" % res) except (socket.error, IOError), e: raise ClientConnectionError("Unable to connect to " @@ -216,11 +216,6 @@ class Client(BaseClient): :retval The newly-stored image's metadata. """ - if not image_data and 'location' not in image_meta.keys(): - raise exception.Invalid("You must either specify a location " - "for the image or supply the actual " - "image data when adding an image to " - "Glance") if image_data: if hasattr(image_data, 'read'): # TODO(jaypipes): This is far from efficient. Implement @@ -242,17 +237,22 @@ class Client(BaseClient): res = self.do_request("POST", "/images", body, headers) data = json.loads(res.read()) - return data['image']['id'] + return data['image'] - def update_image(self, image_id, image_metadata): + def update_image(self, image_id, image_meta=None, image_data=None): """ Updates Glance's information about an image """ - if 'image' not in image_metadata.keys(): - image_metadata = dict(image=image_metadata) - body = json.dumps(image_metadata) - self.do_request("PUT", "/images/%s" % image_id, body) - return True + if image_meta: + if 'image' not in image_meta: + image_meta = dict(image=image_meta) + + headers = util.image_meta_to_http_headers(image_meta or {}) + + body = image_data # TODO(sirp): more work here? + res = self.do_request("PUT", "/images/%s" % image_id, body, headers) + data = json.loads(res.read()) + return data['image'] def delete_image(self, image_id): """ diff --git a/glance/registry/db/sqlalchemy/models.py b/glance/registry/db/sqlalchemy/models.py index e1b28cfa61..d528b2ce17 100644 --- a/glance/registry/db/sqlalchemy/models.py +++ b/glance/registry/db/sqlalchemy/models.py @@ -159,7 +159,7 @@ class Image(BASE, ModelBase): @validates('status') def validate_status(self, key, status): - if not status in ('available', 'pending', 'disabled'): + if not status in ('active', 'queued', 'killed', 'saving'): raise exception.Invalid("Invalid status '%s' for image." % status) return status diff --git a/glance/registry/server.py b/glance/registry/server.py index 6ff30f8fbb..baeceae9f7 100644 --- a/glance/registry/server.py +++ b/glance/registry/server.py @@ -106,7 +106,7 @@ class ImageController(wsgi.Controller): image_data = json.loads(req.body)['image'] # Ensure the image has a status set - image_data.setdefault('status', 'available') + image_data.setdefault('status', 'active') context = None try: diff --git a/glance/server.py b/glance/server.py index 5d130440c6..d542c32364 100644 --- a/glance/server.py +++ b/glance/server.py @@ -162,7 +162,69 @@ class Controller(wsgi.Controller): util.inject_image_meta_into_headers(res, image) return req.get_response(res) + + def _reserve(self, req): + image_meta = util.get_image_meta_from_headers(req) + image_meta['status'] = 'queued' + + try: + image_meta = registry.add_image_metadata(image_meta) + return image_meta + except exception.Duplicate: + msg = "An image with identifier %s already exists"\ + % image_meta['id'] + logging.error(msg) + raise HTTPConflict(msg, request=req) + except exception.Invalid: + raise HTTPBadRequest() + + def _upload(self, req, image_meta): + content_type = req.headers.get('content-type', 'notset') + if content_type != 'application/octet-stream': + raise HTTPBadRequest( + "Content-Type must be application/octet-stream") + + image_store = req.headers.get( + 'x-image-meta-store', FLAGS.default_store) + + store = self.get_store_or_400(req, image_store) + + image_meta['status'] = 'saving' + registry.update_image_metadata(image_meta['id'], image_meta) + + try: + location = store.add(image_meta['id'], req.body) + return location + except exception.Duplicate, e: + logging.error("Error adding image to store: %s", str(e)) + raise HTTPConflict(str(e), request=req) + + def _activate(self, req, image_meta, location): + image_meta['location'] = location + image_meta['status'] = 'active' + registry.update_image_metadata(image_meta['id'], image_meta) + + def _kill(self, req, image_meta): + image_meta['status'] = 'killed' + registry.update_image_metadata(image_meta['id'], image_meta) + def create(self, req): + image_meta = self._reserve(req) + if req.body: + try: + location = self._upload(req, image_meta) + self._activate(req, image_meta, location) + except: + self._kill(req, image_meta) + raise + else: + if req.headers.has_key('x-image-meta-location'): + location = req.headers['x-image-meta-location'] + self._activate(req, image_meta, location) + + return dict(image=image_meta) + + def old_create(self, req): """ Adds a new image to Glance. The body of the request may be a mime-encoded image data. Metadata about the image is sent via @@ -242,6 +304,25 @@ class Controller(wsgi.Controller): return HTTPBadRequest() def update(self, req, id): + orig_image_meta = self.get_image_meta_or_404(req, id) + new_image_meta = util.get_image_meta_from_headers(req) + + if req.body and (orig_image_meta['status'] != 'queued'): + raise HTTPConflict("Cannot upload to an unqueued image") + + image_meta = registry.update_image_metadata(id, new_image_meta) + + if req.body: + try: + location = self._upload(req, image_meta) + self._activate(req, image_meta, location) + except: + self._kill(req, image_meta) + raise + + return dict(image=image_meta) + + def old_update(self, req, id): """ Updates an existing image with the registry. diff --git a/tests/stubs.py b/tests/stubs.py index 02ee3b89bb..02fa7fa70d 100644 --- a/tests/stubs.py +++ b/tests/stubs.py @@ -278,7 +278,7 @@ def stub_out_registry_db_image_api(stubs): FIXTURES = [ {'id': 1, 'name': 'fake image #1', - 'status': 'available', + 'status': 'active', 'type': 'kernel', 'is_public': False, 'created_at': datetime.datetime.utcnow(), @@ -290,7 +290,7 @@ def stub_out_registry_db_image_api(stubs): 'properties': []}, {'id': 2, 'name': 'fake image #2', - 'status': 'available', + 'status': 'active', 'type': 'kernel', 'is_public': True, 'created_at': datetime.datetime.utcnow(), @@ -301,7 +301,7 @@ def stub_out_registry_db_image_api(stubs): 'location': "file:///tmp/glance-tests/2", 'properties': []}] - VALID_STATUSES = ('available', 'disabled', 'pending') + VALID_STATUSES = ('active', 'killed', 'queued', 'saving') def __init__(self): self.images = FakeDatastore.FIXTURES @@ -316,7 +316,7 @@ def stub_out_registry_db_image_api(stubs): values['id']) if 'status' not in values.keys(): - values['status'] = 'available' + values['status'] = 'active' else: if not values['status'] in self.VALID_STATUSES: raise exception.Invalid("Invalid status '%s' for image" % diff --git a/tests/unit/test_api.py b/tests/unit/test_api.py index 34f83980d9..a81ba6ba03 100644 --- a/tests/unit/test_api.py +++ b/tests/unit/test_api.py @@ -15,6 +15,7 @@ # License for the specific language governing permissions and limitations # under the License. +import httplib import json import unittest @@ -87,7 +88,7 @@ class TestRegistryAPI(unittest.TestCase): 'name': 'fake image #2', 'is_public': True, 'type': 'kernel', - 'status': 'available' + 'status': 'active' } req = webob.Request.blank('/images/detail') res = req.get_response(rserver.API()) @@ -125,7 +126,7 @@ class TestRegistryAPI(unittest.TestCase): self.assertEquals(3, res_dict['image']['id']) # Test status was updated properly - self.assertEquals('available', res_dict['image']['status']) + self.assertEquals('active', res_dict['image']['status']) def test_create_image_with_bad_status(self): """Tests proper exception is raised if a bad status is set""" @@ -251,7 +252,7 @@ class TestGlanceAPI(unittest.TestCase): self.stubs.UnsetAll() def test_add_image_no_location_no_image_as_body(self): - """Tests raises BadRequest for no body and no loc header""" + """Tests creates a queued image for no body and no loc header""" fixture_headers = {'x-image-meta-store': 'file', 'x-image-meta-name': 'fake image #3'} @@ -260,7 +261,10 @@ class TestGlanceAPI(unittest.TestCase): for k,v in fixture_headers.iteritems(): req.headers[k] = v res = req.get_response(server.API()) - self.assertEquals(res.status_int, webob.exc.HTTPBadRequest.code) + self.assertEquals(res.status_int, httplib.OK) + + res_body = json.loads(res.body)['image'] + self.assertEquals('queued', res_body['status']) def test_add_image_bad_store(self): """Tests raises BadRequest for invalid store header""" diff --git a/tests/unit/test_clients.py b/tests/unit/test_clients.py index 610ff7aaf8..e46fb77251 100644 --- a/tests/unit/test_clients.py +++ b/tests/unit/test_clients.py @@ -78,7 +78,7 @@ class TestRegistryClient(unittest.TestCase): 'name': 'fake image #2', 'is_public': True, 'type': 'kernel', - 'status': 'available', + 'status': 'active', 'size': 19, 'location': "file:///tmp/glance-tests/2", 'properties': {}} @@ -87,7 +87,7 @@ class TestRegistryClient(unittest.TestCase): 'name': 'fake image #2', 'is_public': True, 'type': 'kernel', - 'status': 'available', + 'status': 'active', 'size': 19, 'location': "file:///tmp/glance-tests/2", 'properties': {}} @@ -104,7 +104,7 @@ class TestRegistryClient(unittest.TestCase): 'name': 'fake image #2', 'is_public': True, 'type': 'kernel', - 'status': 'available', + 'status': 'active', 'size': 19, 'location': "file:///tmp/glance-tests/2", 'properties': {}} @@ -113,7 +113,7 @@ class TestRegistryClient(unittest.TestCase): 'name': 'fake image #2', 'is_public': True, 'type': 'kernel', - 'status': 'available', + 'status': 'active', 'size': 19, 'location': "file:///tmp/glance-tests/2", 'properties': {}} @@ -152,7 +152,7 @@ class TestRegistryClient(unittest.TestCase): # Test status was updated properly self.assertTrue('status' in data.keys()) - self.assertEquals('available', data['status']) + self.assertEquals('active', data['status']) def test_add_image_with_properties(self): """Tests that we can add image metadata with properties""" @@ -181,7 +181,7 @@ class TestRegistryClient(unittest.TestCase): # Test status was updated properly self.assertTrue('status' in new_image.keys()) - self.assertEquals('available', new_image['status']) + self.assertEquals('active', new_image['status']) def test_add_image_already_exists(self): """Tests proper exception is raised if image with ID already exists""" @@ -293,7 +293,7 @@ class TestClient(unittest.TestCase): 'name': 'fake image #2', 'is_public': True, 'type': 'kernel', - 'status': 'available', + 'status': 'active', 'size': 19, 'location': "file:///tmp/glance-tests/2", 'properties': {}} @@ -330,7 +330,7 @@ class TestClient(unittest.TestCase): 'name': 'fake image #2', 'is_public': True, 'type': 'kernel', - 'status': 'available', + 'status': 'active', 'size': 19, 'location': "file:///tmp/glance-tests/2", 'properties': {}} @@ -339,7 +339,7 @@ class TestClient(unittest.TestCase): 'name': 'fake image #2', 'is_public': True, 'type': 'kernel', - 'status': 'available', + 'status': 'active', 'size': 19, 'location': "file:///tmp/glance-tests/2", 'properties': {}} @@ -356,7 +356,7 @@ class TestClient(unittest.TestCase): 'name': 'fake image #2', 'is_public': True, 'type': 'kernel', - 'status': 'available', + 'status': 'active', 'size': 19, 'location': "file:///tmp/glance-tests/2", 'properties': {}} @@ -365,7 +365,7 @@ class TestClient(unittest.TestCase): 'name': 'fake image #2', 'is_public': True, 'type': 'kernel', - 'status': 'available', + 'status': 'active', 'size': 19, 'location': "file:///tmp/glance-tests/2", 'properties': {}} @@ -383,15 +383,14 @@ class TestClient(unittest.TestCase): 42) def test_add_image_without_location_or_raw_data(self): - """Tests client throws Invalid if missing both location and raw data""" + """Tests client returns image as queued""" fixture = {'name': 'fake public image', 'is_public': True, 'type': 'kernel' } - - self.assertRaises(exception.Invalid, - self.client.add_image, - fixture) + + image_meta = self.client.add_image(fixture) + self.assertEquals('queued', image_meta['status']) def test_add_image_basic(self): """Tests that we can add image metadata and returns the new id""" @@ -402,10 +401,11 @@ class TestClient(unittest.TestCase): 'location': "file:///tmp/glance-tests/2", } - new_id = self.client.add_image(fixture) + new_image = self.client.add_image(fixture) + new_image_id = new_image['id'] # Test ID auto-assigned properly - self.assertEquals(3, new_id) + self.assertEquals(3, new_image_id) # Test all other attributes set data = self.client.get_image_meta(3) @@ -415,7 +415,7 @@ class TestClient(unittest.TestCase): # Test status was updated properly self.assertTrue('status' in data.keys()) - self.assertEquals('available', data['status']) + self.assertEquals('active', data['status']) def test_add_image_with_properties(self): """Tests that we can add image metadata with properties""" @@ -434,10 +434,11 @@ class TestClient(unittest.TestCase): 'properties': {'distro': 'Ubuntu 10.04 LTS'} } - new_id = self.client.add_image(fixture) + new_image = self.client.add_image(fixture) + new_image_id = new_image['id'] # Test ID auto-assigned properly - self.assertEquals(3, new_id) + self.assertEquals(3, new_image_id) # Test all other attributes set data = self.client.get_image_meta(3) @@ -446,8 +447,8 @@ class TestClient(unittest.TestCase): self.assertEquals(v, data[k]) # Test status was updated properly - self.assertTrue('status' in data.keys()) - self.assertEquals('available', data['status']) + self.assertTrue('status' in data) + self.assertEquals('active', data['status']) def test_add_image_already_exists(self): """Tests proper exception is raised if image with ID already exists""" @@ -474,11 +475,8 @@ class TestClient(unittest.TestCase): 'location': "file:///tmp/glance-tests/2", } - new_id = self.client.add_image(fixture) - - data = self.client.get_image_meta(new_id) - - self.assertEquals(data['status'], 'available') + new_image = self.client.add_image(fixture) + self.assertEquals(new_image['status'], 'active') def test_add_image_with_image_data_as_string(self): """Tests can add image by passing image data as string""" @@ -491,9 +489,9 @@ class TestClient(unittest.TestCase): image_data_fixture = r"chunk0000remainder" - new_id = self.client.add_image(fixture, image_data_fixture) - - self.assertEquals(3, new_id) + new_image = self.client.add_image(fixture, image_data_fixture) + new_image_id = new_image['id'] + self.assertEquals(3, new_image_id) new_meta, new_image_chunks = self.client.get_image(3) @@ -525,9 +523,9 @@ class TestClient(unittest.TestCase): tmp_file.write(image_data_fixture) tmp_file.close() - new_id = self.client.add_image(fixture, open(tmp_image_filepath)) - - self.assertEquals(3, new_id) + new_image = self.client.add_image(fixture, open(tmp_image_filepath)) + new_image_id = new_image['id'] + self.assertEquals(3, new_image_id) if os.path.exists(tmp_image_filepath): os.unlink(tmp_image_filepath) diff --git a/tests/unit/test_registry_api.py b/tests/unit/test_registry_api.py index 57f2c6e50e..9d044b30bc 100644 --- a/tests/unit/test_registry_api.py +++ b/tests/unit/test_registry_api.py @@ -80,7 +80,7 @@ class TestImageController(unittest.TestCase): 'name': 'fake image #2', 'is_public': True, 'type': 'kernel', - 'status': 'available' + 'status': 'active' } req = webob.Request.blank('/images/detail') res = req.get_response(server.API()) @@ -118,7 +118,7 @@ class TestImageController(unittest.TestCase): self.assertEquals(3, res_dict['image']['id']) # Test status was updated properly - self.assertEquals('available', res_dict['image']['status']) + self.assertEquals('active', res_dict['image']['status']) def test_create_image_with_bad_status(self): """Tests proper exception is raised if a bad status is set""" From cb5d4fd2e81537f0d6ad0e420ab9aa2610e78c54 Mon Sep 17 00:00:00 2001 From: Rick Harris Date: Tue, 4 Jan 2011 18:16:53 -0600 Subject: [PATCH 3/6] removing old methods --- glance/server.py | 127 ++++++++++++++--------------------------------- 1 file changed, 36 insertions(+), 91 deletions(-) diff --git a/glance/server.py b/glance/server.py index d542c32364..a1bab054da 100644 --- a/glance/server.py +++ b/glance/server.py @@ -209,6 +209,37 @@ class Controller(wsgi.Controller): registry.update_image_metadata(image_meta['id'], image_meta) def create(self, req): + """ + Adds a new image to Glance. Three scenarios exist when creating an + image: + + 1. If the image data is available for upload, create can be passed the + image data as the request body and the metadata as the request + headers. The image will initially be 'queued', during upload it + will be in the 'saving' status, and then 'killed' or 'active' + depending on whether the upload completed successfully. + + 2. If the image data exists somewhere else, you can pass in the source + using the x-image-meta-location header + + 3. If the image data is not available yet, but you'd like reserve a + spot for it, you can omit the data and a record will be created in + the 'queued' state. This exists primarily to maintain backwards + compatibility with OpenStack/Rackspace API semantics. + + The request body *must* be encoded as application/octet-stream, + otherwise an HTTPBadRequest is returned. + + Upon a successful save of the image data and metadata, a response + containing metadata about the image is returned, including its + opaque identifier. + + :param request: The WSGI/Webob Request object + + :raises HTTPBadRequest if no x-image-meta-location is missing + and the request body is not application/octet-stream + image data. + """ image_meta = self._reserve(req) if req.body: try: @@ -224,86 +255,16 @@ class Controller(wsgi.Controller): return dict(image=image_meta) - def old_create(self, req): + def update(self, req, id): """ - Adds a new image to Glance. The body of the request may be a - mime-encoded image data. Metadata about the image is sent via - HTTP Headers. - - If the metadata about the image does not include a location - to find the image, or if the image location is not valid, - the request body *must* be encoded as application/octet-stream - and be the image data itself, otherwise an HTTPBadRequest is - returned. - - Upon a successful save of the image data and metadata, a response - containing metadata about the image is returned, including its - opaque identifier. + Updates an existing image with the registry. :param request: The WSGI/Webob Request object + :param id: The opaque image identifier + + :retval Returns the updated image information as a mapping - :raises HTTPBadRequest if no x-image-meta-location is missing - and the request body is not application/octet-stream - image data. """ - - # Verify the request and headers before we generate a new id - - image_in_body = False - image_store = None - header_keys = [k.lower() for k in req.headers.keys()] - if 'x-image-meta-location' not in header_keys: - if ('content-type' not in header_keys or - req.headers['content-type'] != 'application/octet-stream'): - raise HTTPBadRequest("Image location was not specified in " - "headers and the request body was not " - "mime-encoded as application/" - "octet-stream.", request=req) - else: - if 'x-image-meta-store' in header_keys: - image_store = req.headers['x-image-meta-store'] - image_status = 'pending' # set to available when stored... - image_in_body = True - else: - image_location = req.headers['x-image-meta-location'] - image_store = get_store_from_location(image_location) - image_status = 'available' - - # If image is the request body, validate that the requested - # or default store is capable of storing the image data... - if not image_store: - image_store = FLAGS.default_store - if image_in_body: - store = self.get_store_or_400(req, image_store) - - image_meta = util.get_image_meta_from_headers(req) - - image_meta['status'] = image_status - image_meta['store'] = image_store - try: - image_meta = registry.add_image_metadata(image_meta) - - if image_in_body: - try: - location = store.add(image_meta['id'], req.body) - except exception.Duplicate, e: - logging.error("Error adding image to store: %s", str(e)) - return HTTPConflict(str(e), request=req) - image_meta['status'] = 'available' - image_meta['location'] = location - registry.update_image_metadata(image_meta['id'], image_meta) - - return dict(image=image_meta) - - except exception.Duplicate: - msg = "An image with identifier %s already exists"\ - % image_meta['id'] - logging.error(msg) - return HTTPConflict(msg, request=req) - except exception.Invalid: - return HTTPBadRequest() - - def update(self, req, id): orig_image_meta = self.get_image_meta_or_404(req, id) new_image_meta = util.get_image_meta_from_headers(req) @@ -322,22 +283,6 @@ class Controller(wsgi.Controller): return dict(image=image_meta) - def old_update(self, req, id): - """ - Updates an existing image with the registry. - - :param request: The WSGI/Webob Request object - :param id: The opaque image identifier - - :retval Returns the updated image information as a mapping, - - """ - image = self.get_image_meta_or_404(req, id) - - image_data = json.loads(req.body)['image'] - updated_image = registry.update_image_metadata(id, image_data) - return dict(image=updated_image) - def delete(self, req, id): """ Deletes the image and all its chunks from the Glance From 19e411bc48ff5a9cec34adccfbc337ce472c01fa Mon Sep 17 00:00:00 2001 From: Rick Harris Date: Thu, 6 Jan 2011 19:14:53 -0600 Subject: [PATCH 4/6] Fixing eventlet-raise issue --- glance/client.py | 13 +++++---- glance/registry/client.py | 8 ++++-- glance/registry/db/sqlalchemy/api.py | 4 ++- glance/server.py | 40 +++++++++++++++++++--------- glance/store/filesystem.py | 1 - 5 files changed, 44 insertions(+), 22 deletions(-) diff --git a/glance/client.py b/glance/client.py index 06bc83893f..d648675a73 100644 --- a/glance/client.py +++ b/glance/client.py @@ -120,11 +120,11 @@ class BaseClient(object): elif status_code == httplib.NOT_FOUND: raise exception.NotFound elif status_code == httplib.CONFLICT: - raise exception.Duplicate(res) + raise exception.Duplicate(res.read()) elif status_code == httplib.BAD_REQUEST: - raise exception.BadInputError(res) + raise exception.BadInputError(res.read()) else: - raise Exception("Unknown error occurred! %s" % res) + raise Exception("Unknown error occurred! %s" % res.__dict__) except (socket.error, IOError), e: raise ClientConnectionError("Unable to connect to " @@ -203,12 +203,12 @@ class Client(BaseClient): image = util.get_image_meta_from_headers(res) return image - def add_image(self, image_meta, image_data=None): + def add_image(self, image_meta=None, image_data=None): """ Tells Glance about an image's metadata as well as optionally the image_data itself - :param image_meta: Mapping of information about the + :param image_meta: Optional Mapping of information about the image :param image_data: Optional string of raw image data or file-like object that can be @@ -216,6 +216,9 @@ class Client(BaseClient): :retval The newly-stored image's metadata. """ + if image_meta is None: + image_meta = {} + if image_data: if hasattr(image_data, 'read'): # TODO(jaypipes): This is far from efficient. Implement diff --git a/glance/registry/client.py b/glance/registry/client.py index b268f488a4..af78a7a33e 100644 --- a/glance/registry/client.py +++ b/glance/registry/client.py @@ -93,9 +93,13 @@ class RegistryClient(BaseClient): """ if 'image' not in image_metadata.keys(): image_metadata = dict(image=image_metadata) + body = json.dumps(image_metadata) - self.do_request("PUT", "/images/%s" % image_id, body) - return True + + res = self.do_request("PUT", "/images/%s" % image_id, body) + data = json.loads(res.read()) + image = data['image'] + return image def delete_image(self, image_id): """ diff --git a/glance/registry/db/sqlalchemy/api.py b/glance/registry/db/sqlalchemy/api.py index 3046bf30d4..d0e1e84311 100644 --- a/glance/registry/db/sqlalchemy/api.py +++ b/glance/registry/db/sqlalchemy/api.py @@ -129,7 +129,9 @@ def _image_update(_context, values, image_id): with session.begin(): _drop_protected_attrs(models.Image, values) - values['size'] = int(values['size']) + if 'size' in values: + values['size'] = int(values['size']) + values['is_public'] = bool(values.get('is_public', False)) properties = values.pop('properties', {}) diff --git a/glance/server.py b/glance/server.py index a1bab054da..62ab86a4fe 100644 --- a/glance/server.py +++ b/glance/server.py @@ -162,7 +162,6 @@ class Controller(wsgi.Controller): util.inject_image_meta_into_headers(res, image) return req.get_response(res) - def _reserve(self, req): image_meta = util.get_image_meta_from_headers(req) image_meta['status'] = 'queued' @@ -207,6 +206,30 @@ class Controller(wsgi.Controller): def _kill(self, req, image_meta): image_meta['status'] = 'killed' registry.update_image_metadata(image_meta['id'], image_meta) + + def _safe_kill(self, req, image_meta): + """Mark image killed without raising exceptions if it fails. + + Since _kill is meant to be called from exceptions handlers, it should + not raise itself, rather it should just log its error. + """ + try: + self._kill(req, image_meta) + except Exception, e: + logging.error("Unable to kill image %s: %s", + image_meta['id'], repr(e)) + + def _upload_and_activate(self, req, image_meta): + try: + location = self._upload(req, image_meta) + self._activate(req, image_meta, location) + except Exception, e: + # NOTE(sirp): _safe_kill uses httplib which, in turn, uses + # Eventlet's GreenSocket. Eventlet subsequently clears exceptions + # by calling `sys.exc_clear()`. This is why we have to `raise e` + # instead of `raise` + self._safe_kill(req, image_meta) + raise e def create(self, req): """ @@ -241,13 +264,9 @@ class Controller(wsgi.Controller): image data. """ image_meta = self._reserve(req) + if req.body: - try: - location = self._upload(req, image_meta) - self._activate(req, image_meta, location) - except: - self._kill(req, image_meta) - raise + self._upload_and_activate(req, image_meta) else: if req.headers.has_key('x-image-meta-location'): location = req.headers['x-image-meta-location'] @@ -274,12 +293,7 @@ class Controller(wsgi.Controller): image_meta = registry.update_image_metadata(id, new_image_meta) if req.body: - try: - location = self._upload(req, image_meta) - self._activate(req, image_meta, location) - except: - self._kill(req, image_meta) - raise + self._upload_and_activate(req, image_meta) return dict(image=image_meta) diff --git a/glance/store/filesystem.py b/glance/store/filesystem.py index 7c380ef2c2..88b9bee282 100644 --- a/glance/store/filesystem.py +++ b/glance/store/filesystem.py @@ -114,7 +114,6 @@ class FilesystemBackend(glance.store.Backend): :retval The location that was written, with file:// scheme prepended """ - datadir = FLAGS.filesystem_store_datadir if not os.path.exists(datadir): From 6880a7a764239d445b93062b4d7ee8f52f677929 Mon Sep 17 00:00:00 2001 From: Rick Harris Date: Fri, 7 Jan 2011 00:13:33 -0600 Subject: [PATCH 5/6] PEP-8 Fixes --- glance/client.py | 2 +- glance/common/utils.py | 3 +-- glance/registry/db/sqlalchemy/api.py | 28 ++++++++++++------------- glance/registry/db/sqlalchemy/models.py | 20 +++++++++--------- glance/server.py | 12 +++++------ glance/store/__init__.py | 10 ++++----- glance/store/backends/__init__.py | 10 ++++----- 7 files changed, 40 insertions(+), 45 deletions(-) diff --git a/glance/client.py b/glance/client.py index d648675a73..906b40bc92 100644 --- a/glance/client.py +++ b/glance/client.py @@ -252,7 +252,7 @@ class Client(BaseClient): headers = util.image_meta_to_http_headers(image_meta or {}) - body = image_data # TODO(sirp): more work here? + body = image_data res = self.do_request("PUT", "/images/%s" % image_id, body, headers) data = json.loads(res.read()) return data['image'] diff --git a/glance/common/utils.py b/glance/common/utils.py index f0fd3eec94..9bdc3a826c 100644 --- a/glance/common/utils.py +++ b/glance/common/utils.py @@ -141,8 +141,7 @@ def generate_uid(topic, size=8): def generate_mac(): mac = [0x02, 0x16, 0x3e, random.randint(0x00, 0x7f), - random.randint(0x00, 0xff), random.randint(0x00, 0xff) - ] + random.randint(0x00, 0xff), random.randint(0x00, 0xff)] return ':'.join(map(lambda x: "%02x" % x, mac)) diff --git a/glance/registry/db/sqlalchemy/api.py b/glance/registry/db/sqlalchemy/api.py index d0e1e84311..68bc27b6d3 100644 --- a/glance/registry/db/sqlalchemy/api.py +++ b/glance/registry/db/sqlalchemy/api.py @@ -65,11 +65,11 @@ def image_destroy(_context, image_id): def image_get(context, image_id): session = get_session() try: - return session.query(models.Image - ).options(joinedload(models.Image.properties) - ).filter_by(deleted=_deleted(context) - ).filter_by(id=image_id - ).one() + return session.query(models.Image).\ + options(joinedload(models.Image.properties)).\ + filter_by(deleted=_deleted(context)).\ + filter_by(id=image_id).\ + one() except exc.NoResultFound: new_exc = exception.NotFound("No model for id %s" % image_id) raise new_exc.__class__, new_exc, sys.exc_info()[2] @@ -77,19 +77,19 @@ def image_get(context, image_id): def image_get_all(context): session = get_session() - return session.query(models.Image - ).options(joinedload(models.Image.properties) - ).filter_by(deleted=_deleted(context) - ).all() + return session.query(models.Image).\ + options(joinedload(models.Image.properties)).\ + filter_by(deleted=_deleted(context)).\ + all() def image_get_all_public(context, public): session = get_session() - return session.query(models.Image - ).options(joinedload(models.Image.properties) - ).filter_by(deleted=_deleted(context) - ).filter_by(is_public=public - ).all() + return session.query(models.Image).\ + options(joinedload(models.Image.properties)).\ + filter_by(deleted=_deleted(context)).\ + filter_by(is_public=public).\ + all() def image_get_by_str(context, str_id): diff --git a/glance/registry/db/sqlalchemy/models.py b/glance/registry/db/sqlalchemy/models.py index 42b57c28c5..2c963b350e 100644 --- a/glance/registry/db/sqlalchemy/models.py +++ b/glance/registry/db/sqlalchemy/models.py @@ -58,18 +58,18 @@ class ModelBase(object): """Get all objects of this type""" if not session: session = get_session() - return session.query(cls - ).filter_by(deleted=deleted - ).all() + return session.query(cls).\ + filter_by(deleted=deleted).\ + all() @classmethod def count(cls, session=None, deleted=False): """Count objects of this type""" if not session: session = get_session() - return session.query(cls - ).filter_by(deleted=deleted - ).count() + return session.query(cls).\ + filter_by(deleted=deleted).\ + count() @classmethod def find(cls, obj_id, session=None, deleted=False): @@ -77,10 +77,10 @@ class ModelBase(object): if not session: session = get_session() try: - return session.query(cls - ).filter_by(id=obj_id - ).filter_by(deleted=deleted - ).one() + return session.query(cls).\ + filter_by(id=obj_id).\ + filter_by(deleted=deleted).\ + one() except exc.NoResultFound: new_exc = exception.NotFound("No model for id %s" % obj_id) raise new_exc.__class__, new_exc, sys.exc_info()[2] diff --git a/glance/server.py b/glance/server.py index 62ab86a4fe..ea76ab3605 100644 --- a/glance/server.py +++ b/glance/server.py @@ -165,7 +165,7 @@ class Controller(wsgi.Controller): def _reserve(self, req): image_meta = util.get_image_meta_from_headers(req) image_meta['status'] = 'queued' - + try: image_meta = registry.add_image_metadata(image_meta) return image_meta @@ -185,7 +185,7 @@ class Controller(wsgi.Controller): image_store = req.headers.get( 'x-image-meta-store', FLAGS.default_store) - + store = self.get_store_or_400(req, image_store) image_meta['status'] = 'saving' @@ -206,10 +206,10 @@ class Controller(wsgi.Controller): def _kill(self, req, image_meta): image_meta['status'] = 'killed' registry.update_image_metadata(image_meta['id'], image_meta) - + def _safe_kill(self, req, image_meta): """Mark image killed without raising exceptions if it fails. - + Since _kill is meant to be called from exceptions handlers, it should not raise itself, rather it should just log its error. """ @@ -219,7 +219,7 @@ class Controller(wsgi.Controller): logging.error("Unable to kill image %s: %s", image_meta['id'], repr(e)) - def _upload_and_activate(self, req, image_meta): + def _upload_and_activate(self, req, image_meta): try: location = self._upload(req, image_meta) self._activate(req, image_meta, location) @@ -268,7 +268,7 @@ class Controller(wsgi.Controller): if req.body: self._upload_and_activate(req, image_meta) else: - if req.headers.has_key('x-image-meta-location'): + if 'x-image-meta-location' in req.headers: location = req.headers['x-image-meta-location'] self._activate(req, image_meta, location) diff --git a/glance/store/__init__.py b/glance/store/__init__.py index f1922fe88f..ae0c5ed2b6 100644 --- a/glance/store/__init__.py +++ b/glance/store/__init__.py @@ -56,12 +56,10 @@ def get_backend_class(backend): from glance.store.swift import SwiftBackend from glance.store.filesystem import FilesystemBackend - BACKENDS = { - "file": FilesystemBackend, - "http": HTTPBackend, - "https": HTTPBackend, - "swift": SwiftBackend - } + BACKENDS = {"file": FilesystemBackend, + "http": HTTPBackend, + "https": HTTPBackend, + "swift": SwiftBackend} try: return BACKENDS[backend] diff --git a/glance/store/backends/__init__.py b/glance/store/backends/__init__.py index 121819252c..a90ac4879f 100644 --- a/glance/store/backends/__init__.py +++ b/glance/store/backends/__init__.py @@ -87,12 +87,10 @@ def get_backend_class(backend): from glance.store.backends.http import HTTPBackend from glance.store.backends.swift import SwiftBackend - BACKENDS = { - "file": FilesystemBackend, - "http": HTTPBackend, - "https": HTTPBackend, - "swift": SwiftBackend - } + BACKENDS = {"file": FilesystemBackend, + "http": HTTPBackend, + "https": HTTPBackend, + "swift": SwiftBackend} try: return BACKENDS[backend] From 21cea9c5e61990797edb662ed0a49734c5e6f128 Mon Sep 17 00:00:00 2001 From: Rick Harris Date: Tue, 11 Jan 2011 11:34:48 -0600 Subject: [PATCH 6/6] Updating docs --- doc/source/client.rst | 4 ++++ doc/source/glanceapi.rst | 16 ++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/doc/source/client.rst b/doc/source/client.rst index 012ea34454..a4f014dac0 100644 --- a/doc/source/client.rst +++ b/doc/source/client.rst @@ -133,6 +133,10 @@ The method signature is as follows:: The `image_meta` argument is a mapping containing various image metadata. The `image_data` argument is the disk image data. +If the data is not yet available, but you would like to reserve a slot in +Glance to hold the image, you can create a 'queued' by omitting the +`image_data` parameter. + The list of metadata that `image_meta` can contain are listed below. * `name` diff --git a/doc/source/glanceapi.rst b/doc/source/glanceapi.rst index 41f548c4cd..57b007735a 100644 --- a/doc/source/glanceapi.rst +++ b/doc/source/glanceapi.rst @@ -292,3 +292,19 @@ The list of metadata headers that Glance accepts are listed below. be attached to the image. However, keep in mind that the 8K limit on the size of all HTTP headers sent in a request will effectively limit the number of image properties. + + +Updating an Image +***************** + +Glance will view as image metadata any HTTP header that it receives in a +`PUT` request where the header key is prefixed with the strings +`x-image-meta-` and `x-image-meta-property-`. + +If an image was previously reserved, and thus is in the `queued` state, then +image data can be added by including it as the request body. If the image +already as data associated with it (e.g. not in the `queued` state), then +including a request body will result in a `409 Conflict` exception. + +On success, the `PUT` request will return the image metadata encoded as `HTTP` +headers.