From 1cd050e767c4dd5e9a6664911f4f8e408f437f41 Mon Sep 17 00:00:00 2001 From: John Garbutt Date: Tue, 20 Mar 2012 10:14:58 +0000 Subject: Fixes bug 949038 Make the api calls better fit the standard pattern where read_deleted can be changed using context.read_deleted I have retained the ability to pass read_deleted explicitly. If that is not specified, it uses the value in the context. Note: read_deleted defaults to "no" in the context. The two exceptions are: aggregate_host_get_all aggregate_get_all In this case, it is better for read_deleted=yes to be the default So in this case the context cannot be used, as that would default to read_deleted=no. In this case you must explicity override read_deleted, the context is totally ignored, as before. Change-Id: Idb048a592d8c6b788651d131a3345e70989c0ec4 --- nova/db/api.py | 20 ++++++++++---------- nova/db/sqlalchemy/api.py | 42 ++++++++++++++++++++++-------------------- nova/tests/test_db_api.py | 12 +++++++----- 3 files changed, 39 insertions(+), 35 deletions(-) (limited to 'nova') diff --git a/nova/db/api.py b/nova/db/api.py index 2a38d026f..0b06f87fa 100644 --- a/nova/db/api.py +++ b/nova/db/api.py @@ -1683,14 +1683,14 @@ def aggregate_create(context, values, metadata=None): return IMPL.aggregate_create(context, values, metadata) -def aggregate_get(context, aggregate_id, read_deleted='no'): +def aggregate_get(context, aggregate_id, **kwargs): """Get a specific aggregate by id.""" - return IMPL.aggregate_get(context, aggregate_id, read_deleted) + return IMPL.aggregate_get(context, aggregate_id, **kwargs) -def aggregate_get_by_host(context, host, read_deleted='no'): +def aggregate_get_by_host(context, host, **kwargs): """Get a specific aggregate by host""" - return IMPL.aggregate_get_by_host(context, host, read_deleted) + return IMPL.aggregate_get_by_host(context, host, **kwargs) def aggregate_update(context, aggregate_id, values): @@ -1704,9 +1704,9 @@ def aggregate_delete(context, aggregate_id): return IMPL.aggregate_delete(context, aggregate_id) -def aggregate_get_all(context, read_deleted='yes'): +def aggregate_get_all(context, **kwargs): """Get all aggregates.""" - return IMPL.aggregate_get_all(context, read_deleted) + return IMPL.aggregate_get_all(context, **kwargs) def aggregate_metadata_add(context, aggregate_id, metadata, set_delete=False): @@ -1714,9 +1714,9 @@ def aggregate_metadata_add(context, aggregate_id, metadata, set_delete=False): IMPL.aggregate_metadata_add(context, aggregate_id, metadata, set_delete) -def aggregate_metadata_get(context, aggregate_id, read_deleted='no'): +def aggregate_metadata_get(context, aggregate_id, **kwargs): """Get metadata for the specified aggregate.""" - return IMPL.aggregate_metadata_get(context, aggregate_id, read_deleted) + return IMPL.aggregate_metadata_get(context, aggregate_id, **kwargs) def aggregate_metadata_delete(context, aggregate_id, key): @@ -1729,9 +1729,9 @@ def aggregate_host_add(context, aggregate_id, host): IMPL.aggregate_host_add(context, aggregate_id, host) -def aggregate_host_get_all(context, aggregate_id, read_deleted='yes'): +def aggregate_host_get_all(context, aggregate_id, **kwargs): """Get hosts for the specified aggregate.""" - return IMPL.aggregate_host_get_all(context, aggregate_id, read_deleted) + return IMPL.aggregate_host_get_all(context, aggregate_id, **kwargs) def aggregate_host_delete(context, aggregate_id, host): diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 9eda43941..c6af4b85f 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -4084,10 +4084,8 @@ def sm_volume_get_all(context): ################ -def _aggregate_get_query(context, model_class, id_field, id, - session=None, read_deleted='yes'): - return model_query(context, model_class, session=session, - read_deleted=read_deleted).filter(id_field == id) +def _aggregate_get_query(context, model_class, id_field, id, **kwargs): + return model_query(context, model_class, **kwargs).filter(id_field == id) @require_admin_context @@ -4117,11 +4115,12 @@ def aggregate_create(context, values, metadata=None): @require_admin_context -def aggregate_get(context, aggregate_id, read_deleted='no'): +def aggregate_get(context, aggregate_id, **kwargs): aggregate = _aggregate_get_query(context, models.Aggregate, - models.Aggregate.id, aggregate_id, - read_deleted=read_deleted).first() + models.Aggregate.id, + aggregate_id, + **kwargs).first() if not aggregate: raise exception.AggregateNotFound(aggregate_id=aggregate_id) @@ -4130,17 +4129,18 @@ def aggregate_get(context, aggregate_id, read_deleted='no'): @require_admin_context -def aggregate_get_by_host(context, host, read_deleted='no'): +def aggregate_get_by_host(context, host, **kwargs): aggregate_host = _aggregate_get_query(context, models.AggregateHost, models.AggregateHost.host, host, - read_deleted='no').first() + **kwargs).first() if not aggregate_host: raise exception.AggregateHostNotFound(host=host) - return aggregate_get(context, aggregate_host.aggregate_id, read_deleted) + return aggregate_get(context, aggregate_host.aggregate_id, + **kwargs) @require_admin_context @@ -4183,19 +4183,20 @@ def aggregate_delete(context, aggregate_id): @require_admin_context -def aggregate_get_all(context, read_deleted='yes'): +def aggregate_get_all(context, **kwargs): + if 'read_deleted' not in kwargs: + kwargs['read_deleted'] = 'yes' return model_query(context, models.Aggregate, - read_deleted=read_deleted).all() + **kwargs).all() @require_admin_context @require_aggregate_exists -def aggregate_metadata_get(context, aggregate_id, read_deleted='no'): +def aggregate_metadata_get(context, aggregate_id, **kwargs): rows = model_query(context, models.AggregateMetadata, - read_deleted=read_deleted).\ - filter_by(aggregate_id=aggregate_id).all() + **kwargs).filter_by(aggregate_id=aggregate_id).all() return dict([(r['key'], r['value']) for r in rows]) @@ -4220,12 +4221,12 @@ def aggregate_metadata_delete(context, aggregate_id, key): @require_admin_context @require_aggregate_exists def aggregate_metadata_get_item(context, aggregate_id, key, - session=None, read_deleted='yes'): + session=None): result = _aggregate_get_query(context, models.AggregateMetadata, models.AggregateMetadata.aggregate_id, aggregate_id, session=session, - read_deleted=read_deleted).\ + read_deleted='yes').\ filter_by(key=key).first() if not result: @@ -4270,11 +4271,12 @@ def aggregate_metadata_add(context, aggregate_id, metadata, set_delete=False): @require_admin_context @require_aggregate_exists -def aggregate_host_get_all(context, aggregate_id, read_deleted='yes'): +def aggregate_host_get_all(context, aggregate_id, **kwargs): + if 'read_deleted' not in kwargs: + kwargs['read_deleted'] = 'yes' rows = model_query(context, models.AggregateHost, - read_deleted=read_deleted).\ - filter_by(aggregate_id=aggregate_id).all() + **kwargs).filter_by(aggregate_id=aggregate_id).all() return [r.host for r in rows] diff --git a/nova/tests/test_db_api.py b/nova/tests/test_db_api.py index 9e5e3b0ad..158282742 100644 --- a/nova/tests/test_db_api.py +++ b/nova/tests/test_db_api.py @@ -60,8 +60,8 @@ class DbApiTestCase(test.TestCase): def test_instance_get_all_by_filters(self): args = {'reservation_id': 'a', 'image_ref': 1, 'host': 'host1'} - inst1 = db.instance_create(self.context, args) - inst2 = db.instance_create(self.context, args) + db.instance_create(self.context, args) + db.instance_create(self.context, args) result = db.instance_get_all_by_filters(self.context, {}) self.assertTrue(2, len(result)) @@ -436,7 +436,9 @@ class AggregateDBApiTestCase(test.TestCase): db.aggregate_delete(ctxt, result['id']) expected = db.aggregate_get_all(ctxt, read_deleted='no') self.assertEqual(0, len(expected)) - aggregate = db.aggregate_get(ctxt, result['id'], read_deleted='yes') + + ctxt = context.get_admin_context(read_deleted='yes') + aggregate = db.aggregate_get(ctxt, result['id']) self.assertEqual(aggregate["operational_state"], "dismissed") def test_aggregate_update(self): @@ -650,7 +652,7 @@ class CapacityTestCase(test.TestCase): self.assertEquals(item.free_ram_mb, 1024 - 256) def test_compute_node_set(self): - item = self._create_helper('host1') + self._create_helper('host1') x = db.compute_node_utilization_set(self.ctxt, 'host1', free_ram_mb=2048, free_disk_gb=4096) @@ -672,7 +674,7 @@ class CapacityTestCase(test.TestCase): self.assertEquals(x.running_vms, 5) def test_compute_node_utilization_update(self): - item = self._create_helper('host1') + self._create_helper('host1') x = db.compute_node_utilization_update(self.ctxt, 'host1', free_ram_mb_delta=-24) -- cgit