From 24884636dfe7990e3918c7a55a85bba833914349 Mon Sep 17 00:00:00 2001 From: Roman Podolyaka Date: Wed, 24 Jul 2013 16:37:48 +0300 Subject: [PATCH] Fix volume_create()/snapshot_create() DB methods volume_create() and snapshot_create() DB API methods create entities and return information about them by calling _volume_get() and snapshot_get() methods. There is no need to create a new transaction to call the corresponding 'getter'. Unfortunately, fixing this breaks two existing tests, which rely on the current behaviour of these DB API methods: both volume_create() and snapshot_create() raise exceptions, nevertheless new entities are created. Blueprint: db-session-cleanup Change-Id: Idecac70893ec280c149f9227f86a6959d70c15fe --- cinder/db/sqlalchemy/api.py | 4 +- cinder/tests/test_volume.py | 155 +++++++++++++++++------------------- 2 files changed, 76 insertions(+), 83 deletions(-) diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py index 83fc5c10b..749b36eda 100644 --- a/cinder/db/sqlalchemy/api.py +++ b/cinder/db/sqlalchemy/api.py @@ -1006,7 +1006,7 @@ def volume_create(context, values): with session.begin(): volume_ref.save(session=session) - return _volume_get(context, values['id'], session=session) + return _volume_get(context, values['id'], session=session) @require_admin_context @@ -1330,7 +1330,7 @@ def snapshot_create(context, values): with session.begin(): snapshot_ref.save(session=session) - return _snapshot_get(context, values['id'], session=session) + return _snapshot_get(context, values['id'], session=session) @require_admin_context diff --git a/cinder/tests/test_volume.py b/cinder/tests/test_volume.py index 198bf8957..e2cfa0e4e 100644 --- a/cinder/tests/test_volume.py +++ b/cinder/tests/test_volume.py @@ -1139,48 +1139,45 @@ class VolumeTestCase(test.TestCase): self.assertEquals(snap['display_name'], 'test update name') def test_volume_get_active_by_window(self): + ctx = context.get_admin_context(read_deleted="yes") + # Find all all volumes valid within a timeframe window. - try: # Not in window - db.volume_create( - self.context, - { - 'id': 1, - 'host': 'devstack', - 'created_at': datetime.datetime(1, 1, 1, 1, 1, 1), - 'deleted': True, 'status': 'deleted', - 'deleted_at': datetime.datetime(1, 2, 1, 1, 1, 1), - } - ) - except exception.VolumeNotFound: - pass - try: # In - deleted in window - db.volume_create( - self.context, - { - 'id': 2, - 'host': 'devstack', - 'created_at': datetime.datetime(1, 1, 1, 1, 1, 1), - 'deleted': True, 'status': 'deleted', - 'deleted_at': datetime.datetime(1, 3, 10, 1, 1, 1), - } - ) - except exception.VolumeNotFound: - pass + # Not in window + db.volume_create( + ctx, + { + 'id': 1, + 'host': 'devstack', + 'created_at': datetime.datetime(1, 1, 1, 1, 1, 1), + 'deleted': True, 'status': 'deleted', + 'deleted_at': datetime.datetime(1, 2, 1, 1, 1, 1), + } + ) - try: # In - deleted after window - db.volume_create( - self.context, - { - 'id': 3, - 'host': 'devstack', - 'created_at': datetime.datetime(1, 1, 1, 1, 1, 1), - 'deleted': True, 'status': 'deleted', - 'deleted_at': datetime.datetime(1, 5, 1, 1, 1, 1), - } - ) - except exception.VolumeNotFound: - pass + # In - deleted in window + db.volume_create( + ctx, + { + 'id': 2, + 'host': 'devstack', + 'created_at': datetime.datetime(1, 1, 1, 1, 1, 1), + 'deleted': True, 'status': 'deleted', + 'deleted_at': datetime.datetime(1, 3, 10, 1, 1, 1), + } + ) + + # In - deleted after window + db.volume_create( + ctx, + { + 'id': 3, + 'host': 'devstack', + 'created_at': datetime.datetime(1, 1, 1, 1, 1, 1), + 'deleted': True, 'status': 'deleted', + 'deleted_at': datetime.datetime(1, 5, 1, 1, 1, 1), + } + ) # In - created in window db.volume_create( @@ -1212,53 +1209,49 @@ class VolumeTestCase(test.TestCase): self.assertEqual(volumes[2].id, u'4') def test_snapshot_get_active_by_window(self): + ctx = context.get_admin_context(read_deleted="yes") + # Find all all snapshots valid within a timeframe window. vol = db.volume_create(self.context, {'id': 1}) - try: # Not in window - db.snapshot_create( - self.context, - { - 'id': 1, - 'host': 'devstack', - 'volume_id': 1, - 'created_at': datetime.datetime(1, 1, 1, 1, 1, 1), - 'deleted': True, 'status': 'deleted', - 'deleted_at': datetime.datetime(1, 2, 1, 1, 1, 1), - } - ) - except exception.SnapshotNotFound: - pass + # Not in window + db.snapshot_create( + ctx, + { + 'id': 1, + 'host': 'devstack', + 'volume_id': 1, + 'created_at': datetime.datetime(1, 1, 1, 1, 1, 1), + 'deleted': True, 'status': 'deleted', + 'deleted_at': datetime.datetime(1, 2, 1, 1, 1, 1), + } + ) - try: # In - deleted in window - db.snapshot_create( - self.context, - { - 'id': 2, - 'host': 'devstack', - 'volume_id': 1, - 'created_at': datetime.datetime(1, 1, 1, 1, 1, 1), - 'deleted': True, 'status': 'deleted', - 'deleted_at': datetime.datetime(1, 3, 10, 1, 1, 1), - } - ) - except exception.SnapshotNotFound: - pass + # In - deleted in window + db.snapshot_create( + ctx, + { + 'id': 2, + 'host': 'devstack', + 'volume_id': 1, + 'created_at': datetime.datetime(1, 1, 1, 1, 1, 1), + 'deleted': True, 'status': 'deleted', + 'deleted_at': datetime.datetime(1, 3, 10, 1, 1, 1), + } + ) - try: # In - deleted after window - db.snapshot_create( - self.context, - { - 'id': 3, - 'host': 'devstack', - 'volume_id': 1, - 'created_at': datetime.datetime(1, 1, 1, 1, 1, 1), - 'deleted': True, 'status': 'deleted', - 'deleted_at': datetime.datetime(1, 5, 1, 1, 1, 1), - } - ) - except exception.SnapshotNotFound: - pass + # In - deleted after window + db.snapshot_create( + ctx, + { + 'id': 3, + 'host': 'devstack', + 'volume_id': 1, + 'created_at': datetime.datetime(1, 1, 1, 1, 1, 1), + 'deleted': True, 'status': 'deleted', + 'deleted_at': datetime.datetime(1, 5, 1, 1, 1, 1), + } + ) # In - created in window db.snapshot_create( -- 2.45.2