From 689dfae6dba7a319c8f1974929025da638916789 Mon Sep 17 00:00:00 2001 From: Dharini Chandrasekar Date: Sun, 29 Jan 2017 19:37:20 +0000 Subject: [PATCH] Refine migration query added with CI change The query added with CI change [0] was using the python 'and' instead of sqlalchemy's 'and_()'. This patch changes that. This patch also further refines the query to look specifically for visility 'private' instead of '<> public' for accuracy during setting of visibility to 'shared'. [0] I94bc7708b291ce37319539e27b3e88c9a17e1a9f Change-Id: I6851aa0e5ca8cecaff518609c14cd528bca95ade --- .../migrate_repo/versions/045_add_visibility.py | 8 ++++---- .../migrate_repo/versions/045_sqlite_upgrade.sql | 2 +- glance/tests/unit/test_migrations.py | 11 +++++++++++ 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/glance/db/sqlalchemy/migrate_repo/versions/045_add_visibility.py b/glance/db/sqlalchemy/migrate_repo/versions/045_add_visibility.py index 3d4e44c39d..c2724ab936 100644 --- a/glance/db/sqlalchemy/migrate_repo/versions/045_add_visibility.py +++ b/glance/db/sqlalchemy/migrate_repo/versions/045_add_visibility.py @@ -10,7 +10,7 @@ # License for the specific language governing permissions and limitations # under the License. -from sqlalchemy import Column, Enum, Index, MetaData, Table, select, not_ +from sqlalchemy import Column, Enum, Index, MetaData, Table, select, not_, and_ from sqlalchemy.engine import reflection @@ -37,10 +37,10 @@ def upgrade(migrate_engine): images.update().values(visibility='private').where( not_(images.c.is_public)).execute() # NOTE(dharinic): Identify 'shared' images from the above - images.update().values(visibility='shared').where( - images.c.visibility != 'public' and images.c.id.in_(select( + images.update().values(visibility='shared').where(and_( + images.c.visibility == 'private', images.c.id.in_(select( [image_members.c.image_id]).distinct().where( - not_(image_members.c.deleted)))).execute() + not_(image_members.c.deleted))))).execute() insp = reflection.Inspector.from_engine(migrate_engine) for index in insp.get_indexes('images'): diff --git a/glance/db/sqlalchemy/migrate_repo/versions/045_sqlite_upgrade.sql b/glance/db/sqlalchemy/migrate_repo/versions/045_sqlite_upgrade.sql index 6f798bef7c..0e848ccefb 100644 --- a/glance/db/sqlalchemy/migrate_repo/versions/045_sqlite_upgrade.sql +++ b/glance/db/sqlalchemy/migrate_repo/versions/045_sqlite_upgrade.sql @@ -157,6 +157,6 @@ INSERT INTO images ( WHERE is_public=0; UPDATE images SET visibility='private' WHERE visibility='shared'; -UPDATE images SET visibility='shared' WHERE visibility <> 'public' AND id IN (SELECT DISTINCT image_id FROM image_members WHERE deleted != 1); +UPDATE images SET visibility='shared' WHERE visibility='private' AND id IN (SELECT DISTINCT image_id FROM image_members WHERE deleted != 1); DROP TABLE images_backup; diff --git a/glance/tests/unit/test_migrations.py b/glance/tests/unit/test_migrations.py index 5d1b26fca7..7f7e23b9bf 100644 --- a/glance/tests/unit/test_migrations.py +++ b/glance/tests/unit/test_migrations.py @@ -1540,6 +1540,7 @@ class MigrationsMixin(test_migrations.WalkVersionsMixin): can_share=True, id=45) image_members.insert().values(temp).execute() + # adding an image member, but marking it deleted, # for testing 'private' visibility temp = dict(deleted=True, @@ -1550,6 +1551,16 @@ class MigrationsMixin(test_migrations.WalkVersionsMixin): id=451) image_members.insert().values(temp).execute() + # adding an active image member for the 'public' image, + # to test it remains public regardless. + temp = dict(deleted=False, + created_at=now, + image_id='public_id', + member='fake_member_450', + can_share=True, + id=450) + image_members.insert().values(temp).execute() + def _check_045(self, engine, data): # check that after migration, 'visbility' column is introduced images = db_utils.get_table(engine, 'images')