From 9e002a77f2131d3594a2a4029a147beaf37f5b17 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Mon, 16 Aug 2021 11:44:30 +0100 Subject: [PATCH] Resolve compatibility with oslo.db future (redux) Build on change I957d2a9d7c875120bed0022ce1e953b2ec9f97cd by addressing additional issues introduced by future breaking changes in oslo.db. As with this change, the changes stem from the table returned from 'oslo_db.utils.get_table' being no longer bound to a 'Connection' or 'Engine'. Change-Id: Ic28bef6ad4e47a86e423a10e8479b60134cbb0a5 Signed-off-by: Stephen Finucane Related-Bug: #1939716 --- .../db/migrations/test_ocata_contract01.py | 6 +- .../db/migrations/test_ocata_expand01.py | 67 ++++++++++++------- .../db/migrations/test_ocata_migrate01.py | 49 ++++++++------ .../db/migrations/test_train_migrate01.py | 39 ++++++----- 4 files changed, 97 insertions(+), 64 deletions(-) diff --git a/glance/tests/functional/db/migrations/test_ocata_contract01.py b/glance/tests/functional/db/migrations/test_ocata_contract01.py index 9400fe5a66..11d85e2c73 100644 --- a/glance/tests/functional/db/migrations/test_ocata_contract01.py +++ b/glance/tests/functional/db/migrations/test_ocata_contract01.py @@ -41,7 +41,8 @@ class TestOcataContract01Mixin(test_migrations.AlembicMigrationsMixin): min_disk=0, min_ram=0, id='public_id_before_expand') - images.insert().values(public_temp).execute() + with engine.connect() as conn, conn.begin(): + conn.execute(images.insert().values(public_temp)) # inserting a private image record shared_temp = dict(deleted=False, @@ -51,7 +52,8 @@ class TestOcataContract01Mixin(test_migrations.AlembicMigrationsMixin): min_disk=0, min_ram=0, id='private_id_before_expand') - images.insert().values(shared_temp).execute() + with engine.connect() as conn, conn.begin(): + conn.execute(images.insert().values(shared_temp)) data_migrations.migrate(engine=engine, release='ocata') diff --git a/glance/tests/functional/db/migrations/test_ocata_expand01.py b/glance/tests/functional/db/migrations/test_ocata_expand01.py index ef68498778..bc0bde9b56 100644 --- a/glance/tests/functional/db/migrations/test_ocata_expand01.py +++ b/glance/tests/functional/db/migrations/test_ocata_expand01.py @@ -39,7 +39,8 @@ class TestOcataExpand01Mixin(test_migrations.AlembicMigrationsMixin): min_disk=0, min_ram=0, id='public_id_before_expand') - images.insert().values(public_temp).execute() + with engine.connect() as conn, conn.begin(): + conn.execute(images.insert().values(public_temp)) # inserting a private image record shared_temp = dict(deleted=False, @@ -49,7 +50,8 @@ class TestOcataExpand01Mixin(test_migrations.AlembicMigrationsMixin): min_disk=0, min_ram=0, id='private_id_before_expand') - images.insert().values(shared_temp).execute() + with engine.connect() as conn, conn.begin(): + conn.execute(images.insert().values(shared_temp)) def _check_ocata_expand01(self, engine, data): # check that after migration, 'visibility' column is introduced @@ -60,11 +62,12 @@ class TestOcataExpand01Mixin(test_migrations.AlembicMigrationsMixin): self.assertTrue(images.c.visibility.nullable) # tests visibility set to None for existing images - rows = (images.select() - .where(images.c.id.like('%_before_expand')) - .order_by(images.c.id) - .execute() - .fetchall()) + with engine.connect() as conn: + rows = conn.execute( + images.select().where( + images.c.id.like('%_before_expand') + ).order_by(images.c.id) + ).fetchall() self.assertEqual(2, len(rows)) # private image first @@ -76,11 +79,12 @@ class TestOcataExpand01Mixin(test_migrations.AlembicMigrationsMixin): self.assertEqual('public_id_before_expand', rows[1]['id']) self.assertIsNone(rows[1]['visibility']) - self._test_trigger_old_to_new(images) - self._test_trigger_new_to_old(images) + self._test_trigger_old_to_new(engine, images) + self._test_trigger_new_to_old(engine, images) - def _test_trigger_new_to_old(self, images): + def _test_trigger_new_to_old(self, engine, images): now = datetime.datetime.now() + # inserting a public image record after expand public_temp = dict(deleted=False, created_at=now, @@ -89,7 +93,8 @@ class TestOcataExpand01Mixin(test_migrations.AlembicMigrationsMixin): min_disk=0, min_ram=0, id='public_id_new_to_old') - images.insert().values(public_temp).execute() + with engine.connect() as conn, conn.begin(): + conn.execute(images.insert().values(public_temp)) # inserting a private image record after expand shared_temp = dict(deleted=False, @@ -99,7 +104,8 @@ class TestOcataExpand01Mixin(test_migrations.AlembicMigrationsMixin): min_disk=0, min_ram=0, id='private_id_new_to_old') - images.insert().values(shared_temp).execute() + with engine.connect() as conn, conn.begin(): + conn.execute(images.insert().values(shared_temp)) # inserting a shared image record after expand shared_temp = dict(deleted=False, @@ -109,14 +115,16 @@ class TestOcataExpand01Mixin(test_migrations.AlembicMigrationsMixin): min_disk=0, min_ram=0, id='shared_id_new_to_old') - images.insert().values(shared_temp).execute() + with engine.connect() as conn, conn.begin(): + conn.execute(images.insert().values(shared_temp)) # test visibility is set appropriately by the trigger for new images - rows = (images.select() - .where(images.c.id.like('%_new_to_old')) - .order_by(images.c.id) - .execute() - .fetchall()) + with engine.connect() as conn: + rows = conn.execute( + images.select().where( + images.c.id.like('%_new_to_old') + ).order_by(images.c.id) + ).fetchall() self.assertEqual(3, len(rows)) # private image first @@ -132,8 +140,9 @@ class TestOcataExpand01Mixin(test_migrations.AlembicMigrationsMixin): self.assertEqual('shared_id_new_to_old', rows[2]['id']) self.assertEqual('shared', rows[2]['visibility']) - def _test_trigger_old_to_new(self, images): + def _test_trigger_old_to_new(self, engine, images): now = datetime.datetime.now() + # inserting a public image record after expand public_temp = dict(deleted=False, created_at=now, @@ -142,7 +151,9 @@ class TestOcataExpand01Mixin(test_migrations.AlembicMigrationsMixin): min_disk=0, min_ram=0, id='public_id_old_to_new') - images.insert().values(public_temp).execute() + with engine.connect() as conn, conn.begin(): + conn.execute(images.insert().values(public_temp)) + # inserting a private image record after expand shared_temp = dict(deleted=False, created_at=now, @@ -151,13 +162,17 @@ class TestOcataExpand01Mixin(test_migrations.AlembicMigrationsMixin): min_disk=0, min_ram=0, id='private_id_old_to_new') - images.insert().values(shared_temp).execute() + with engine.connect() as conn, conn.begin(): + conn.execute(images.insert().values(shared_temp)) + # tests visibility is set appropriately by the trigger for new images - rows = (images.select() - .where(images.c.id.like('%_old_to_new')) - .order_by(images.c.id) - .execute() - .fetchall()) + with engine.connect() as conn: + rows = conn.execute( + images.select().where( + images.c.id.like('%_old_to_new') + ).order_by(images.c.id) + ).fetchall() + self.assertEqual(2, len(rows)) # private image first self.assertEqual(0, rows[0]['is_public']) diff --git a/glance/tests/functional/db/migrations/test_ocata_migrate01.py b/glance/tests/functional/db/migrations/test_ocata_migrate01.py index 01fbc1f0f2..2ecec9f7fc 100644 --- a/glance/tests/functional/db/migrations/test_ocata_migrate01.py +++ b/glance/tests/functional/db/migrations/test_ocata_migrate01.py @@ -38,7 +38,8 @@ class TestOcataMigrate01Mixin(test_migrations.AlembicMigrationsMixin): min_disk=0, min_ram=0, id='public_id') - images.insert().values(public_temp).execute() + with engine.connect() as conn, conn.begin(): + conn.execute(images.insert().values(public_temp)) # inserting a non-public image record for 'shared' visibility test shared_temp = dict(deleted=False, @@ -48,7 +49,8 @@ class TestOcataMigrate01Mixin(test_migrations.AlembicMigrationsMixin): min_disk=0, min_ram=0, id='shared_id') - images.insert().values(shared_temp).execute() + with engine.connect() as conn, conn.begin(): + conn.execute(images.insert().values(shared_temp)) # inserting a non-public image records for 'private' visibility test private_temp = dict(deleted=False, @@ -58,7 +60,8 @@ class TestOcataMigrate01Mixin(test_migrations.AlembicMigrationsMixin): min_disk=0, min_ram=0, id='private_id_1') - images.insert().values(private_temp).execute() + with engine.connect() as conn, conn.begin(): + conn.execute(images.insert().values(private_temp)) private_temp = dict(deleted=False, created_at=now, @@ -67,7 +70,8 @@ class TestOcataMigrate01Mixin(test_migrations.AlembicMigrationsMixin): min_disk=0, min_ram=0, id='private_id_2') - images.insert().values(private_temp).execute() + with engine.connect() as conn, conn.begin(): + conn.execute(images.insert().values(private_temp)) # adding an active as well as a deleted image member for checking # 'shared' visibility @@ -77,7 +81,8 @@ class TestOcataMigrate01Mixin(test_migrations.AlembicMigrationsMixin): member='fake_member_452', can_share=True, id=45) - image_members.insert().values(temp).execute() + with engine.connect() as conn, conn.begin(): + conn.execute(image_members.insert().values(temp)) temp = dict(deleted=True, created_at=now, @@ -85,7 +90,8 @@ class TestOcataMigrate01Mixin(test_migrations.AlembicMigrationsMixin): member='fake_member_453', can_share=True, id=453) - image_members.insert().values(temp).execute() + with engine.connect() as conn, conn.begin(): + conn.execute(image_members.insert().values(temp)) # adding an image member, but marking it deleted, # for testing 'private' visibility @@ -95,7 +101,8 @@ class TestOcataMigrate01Mixin(test_migrations.AlembicMigrationsMixin): member='fake_member_451', can_share=True, id=451) - image_members.insert().values(temp).execute() + with engine.connect() as conn, conn.begin(): + conn.execute(image_members.insert().values(temp)) # adding an active image member for the 'public' image, # to test it remains public regardless. @@ -105,16 +112,18 @@ class TestOcataMigrate01Mixin(test_migrations.AlembicMigrationsMixin): member='fake_member_450', can_share=True, id=450) - image_members.insert().values(temp).execute() + with engine.connect() as conn, conn.begin(): + conn.execute(image_members.insert().values(temp)) def _check_ocata_expand01(self, engine, data): images = db_utils.get_table(engine, 'images') # check that visibility is null for existing images - rows = (images.select() - .order_by(images.c.id) - .execute() - .fetchall()) + with engine.connect() as conn: + rows = conn.execute( + images.select().order_by(images.c.id) + ).fetchall() + self.assertEqual(4, len(rows)) for row in rows: self.assertIsNone(row['visibility']) @@ -123,10 +132,10 @@ class TestOcataMigrate01Mixin(test_migrations.AlembicMigrationsMixin): data_migrations.migrate(engine) # check that visibility is set appropriately for all images - rows = (images.select() - .order_by(images.c.id) - .execute() - .fetchall()) + with engine.connect() as conn: + rows = conn.execute( + images.select().order_by(images.c.id) + ).fetchall() self.assertEqual(4, len(rows)) # private_id_1 has private visibility self.assertEqual('private_id_1', rows[0]['id']) @@ -167,10 +176,10 @@ class TestOcataMigrate01_EmptyDBMixin(test_migrations.AlembicMigrationsMixin): images = db_utils.get_table(engine, 'images') # check that there are no rows in the images table - rows = (images.select() - .order_by(images.c.id) - .execute() - .fetchall()) + with engine.connect() as conn: + rows = conn.execute( + images.select().order_by(images.c.id) + ).fetchall() self.assertEqual(0, len(rows)) # run data migrations diff --git a/glance/tests/functional/db/migrations/test_train_migrate01.py b/glance/tests/functional/db/migrations/test_train_migrate01.py index 1c4dab60cc..7f0d663d0b 100644 --- a/glance/tests/functional/db/migrations/test_train_migrate01.py +++ b/glance/tests/functional/db/migrations/test_train_migrate01.py @@ -40,7 +40,8 @@ class TestTrainMigrate01Mixin(test_migrations.AlembicMigrationsMixin): min_ram=0, visibility='public', id='image_1') - images.insert().values(image_1).execute() + with engine.connect() as conn, conn.begin(): + conn.execute(images.insert().values(image_1)) image_2 = dict(deleted=False, created_at=now, @@ -49,7 +50,8 @@ class TestTrainMigrate01Mixin(test_migrations.AlembicMigrationsMixin): min_ram=0, visibility='public', id='image_2') - images.insert().values(image_2).execute() + with engine.connect() as conn, conn.begin(): + conn.execute(images.insert().values(image_2)) # adding records to image_locations tables temp = dict(deleted=False, @@ -58,7 +60,8 @@ class TestTrainMigrate01Mixin(test_migrations.AlembicMigrationsMixin): value='image_location_1', meta_data='{"backend": "fast"}', id=1) - image_locations.insert().values(temp).execute() + with engine.connect() as conn, conn.begin(): + conn.execute(image_locations.insert().values(temp)) temp = dict(deleted=False, created_at=now, @@ -66,16 +69,18 @@ class TestTrainMigrate01Mixin(test_migrations.AlembicMigrationsMixin): value='image_location_2', meta_data='{"backend": "cheap"}', id=2) - image_locations.insert().values(temp).execute() + with engine.connect() as conn, conn.begin(): + conn.execute(image_locations.insert().values(temp)) def _check_train_expand01(self, engine, data): image_locations = db_utils.get_table(engine, 'image_locations') # check that meta_data has 'backend' key for existing image_locations - rows = (image_locations.select() - .order_by(image_locations.c.id) - .execute() - .fetchall()) + with engine.connect() as conn: + rows = conn.execute( + image_locations.select().order_by(image_locations.c.id) + ).fetchall() + self.assertEqual(2, len(rows)) for row in rows: self.assertIn('"backend":', row['meta_data']) @@ -84,10 +89,11 @@ class TestTrainMigrate01Mixin(test_migrations.AlembicMigrationsMixin): data_migrations.migrate(engine, release='train') # check that meta_data has 'backend' key replaced with 'store' - rows = (image_locations.select() - .order_by(image_locations.c.id) - .execute() - .fetchall()) + with engine.connect() as conn: + rows = conn.execute( + image_locations.select().order_by(image_locations.c.id) + ).fetchall() + self.assertEqual(2, len(rows)) for row in rows: self.assertNotIn('"backend":', row['meta_data']) @@ -120,10 +126,11 @@ class TestTrainMigrate01_EmptyDBMixin(test_migrations.AlembicMigrationsMixin): images = db_utils.get_table(engine, 'images') # check that there are no rows in the images table - rows = (images.select() - .order_by(images.c.id) - .execute() - .fetchall()) + with engine.connect() as conn: + rows = conn.execute( + images.select().order_by(images.c.id) + ).fetchall() + self.assertEqual(0, len(rows)) # run data migrations