Merge lp:~markmc/nova/sqlalchemy-0.7-migration-fixes into lp:~hudson-openstack/nova/trunk

[Mark McLoughlin]

Just a couple of small changes I needed to get the migrations working with SQLAlchemy 0.7.x on Fedora 16

[Brian Lamar (blamar)]

Thanks for this, been wanting to use latest SQLAlchemy for a bit now. One question, what is the reason/logic for the changes in 036_change_flavor_id_in_migrations.py? At a glance it looks like the logic/queries aren't changing but I most certainly could be missing something.

review: Needs Information

[Mark McLoughlin (markmc)]

Hi Brian,

I guess it's a bit hard to see in the diff, but the new code looks like:

   ...
   itypes = {}
   for instance_type in migrate_engine.execute(instance_types.select()):
       itypes[instance_type.id] = instance_type.flavorid

   for instance_type_id in itypes.keys():
       migrate_engine.execute(migrations.update()\
   ...

i.e. we run the query to get the instance types, collect the results and
drop the lock before doing the updates

The issue with the current code is that the lock appears to be held as
we try to do the updates

Cheers,
Mark.

[Brian Lamar (blamar)]

Understood now. I guess I'm not used to any database locking on selects, but it seems that SQLlite does and SQLALchemy 0.7 somehow exposed that? Anyway, the logic is sound so thanks for the updates to fix this.

review: Approve

[Joshua McKenty (joshua-mckenty)]

lgtm - I see meta.clear() in SQLAlchemy at least back to 0.68

review: Approve

=== modified file 'Authors'
--- Authors	2011-08-23 18:16:04 +0000
+++ Authors	2011-08-24 21:44:18 +0000
@@ -69,6 +69,7 @@
 Lorin Hochstein <[e-mail]>
 Lvov Maxim <[e-mail]>
 Mandell Degerness <[e-mail]>
+Mark McLoughlin <[e-mail]>
 Mark Washenberger <[e-mail]>
 Masanori Itoh <[e-mail]>
 Matt Dietz <[e-mail]>
 
=== modified file 'nova/db/sqlalchemy/migrate_repo/versions/016_make_quotas_key_and_value.py'
--- nova/db/sqlalchemy/migrate_repo/versions/016_make_quotas_key_and_value.py	2011-06-16 19:00:14 +0000
+++ nova/db/sqlalchemy/migrate_repo/versions/016_make_quotas_key_and_value.py	2011-08-24 21:44:18 +0000
@@ -75,8 +75,8 @@
                 )
 
 
-def existing_quotas_table(migrate_engine):
-    return Table('quotas', meta, autoload=True, autoload_with=migrate_engine)
+def quotas_table(migrate_engine, name='quotas'):
+    return Table(name, meta, autoload=True, autoload_with=migrate_engine)
 
 
 def _assert_no_duplicate_project_ids(quotas):
@@ -179,13 +179,18 @@
     # bind migrate_engine to your metadata
     meta.bind = migrate_engine
 
-    old_quotas = existing_quotas_table(migrate_engine)
+    old_quotas = quotas_table(migrate_engine)
     assert_old_quotas_have_no_active_duplicates(migrate_engine, old_quotas)
 
     new_quotas = new_style_quotas_table('quotas_new')
     new_quotas.create()
     convert_forward(migrate_engine, old_quotas, new_quotas)
     old_quotas.drop()
+
+    # clear metadata to work around this:
+    # http://code.google.com/p/sqlalchemy-migrate/issues/detail?id=128
+    meta.clear()
+    new_quotas = quotas_table(migrate_engine, 'quotas_new')
     new_quotas.rename('quotas')
 
 
@@ -193,11 +198,16 @@
     # Operations to reverse the above upgrade go here.
     meta.bind = migrate_engine
 
-    new_quotas = existing_quotas_table(migrate_engine)
+    new_quotas = quotas_table(migrate_engine)
     assert_new_quotas_have_no_active_duplicates(migrate_engine, new_quotas)
 
     old_quotas = old_style_quotas_table('quotas_old')
     old_quotas.create()
     convert_backward(migrate_engine, old_quotas, new_quotas)
     new_quotas.drop()
+
+    # clear metadata to work around this:
+    # http://code.google.com/p/sqlalchemy-migrate/issues/detail?id=128
+    meta.clear()
+    old_quotas = quotas_table(migrate_engine, 'quotas_old')
     old_quotas.rename('quotas')
 
=== modified file 'nova/db/sqlalchemy/migrate_repo/versions/036_change_flavor_id_in_migrations.py'
--- nova/db/sqlalchemy/migrate_repo/versions/036_change_flavor_id_in_migrations.py	2011-08-03 21:44:03 +0000
+++ nova/db/sqlalchemy/migrate_repo/versions/036_change_flavor_id_in_migrations.py	2011-08-24 21:44:18 +0000
@@ -40,13 +40,17 @@
     migrations.create_column(new_instance_type_id)
 
     # Convert flavor_id to instance_type_id
+    itypes = {}
     for instance_type in migrate_engine.execute(instance_types.select()):
-        migrate_engine.execute(migrations.update()\
-                .where(migrations.c.old_flavor_id == instance_type.flavorid)\
-                .values(old_instance_type_id=instance_type.id))
-        migrate_engine.execute(migrations.update()\
-                .where(migrations.c.new_flavor_id == instance_type.flavorid)\
-                .values(new_instance_type_id=instance_type.id))
+        itypes[instance_type.id] = instance_type.flavorid
+
+    for instance_type_id in itypes.keys():
+        migrate_engine.execute(migrations.update()\
+                .where(migrations.c.old_flavor_id == itypes[instance_type_id])\
+                .values(old_instance_type_id=instance_type_id))
+        migrate_engine.execute(migrations.update()\
+                .where(migrations.c.new_flavor_id == itypes[instance_type_id])\
+                .values(new_instance_type_id=instance_type_id))
 
     migrations.c.old_flavor_id.drop()
     migrations.c.new_flavor_id.drop()