Merge lp:~rackspace-titan/nova/postgres-instance-type-id into lp:~hudson-openstack/nova/trunk

[Mark Washenberger]

Convert instance_type_ids in the instances table from strings to integers to enable joins with instance_types. This in particular fixes a problem when using postgresql.

[Rick Harris (rconradharris)]

Looks great.

Tested upgrade and downgrade paths with good and bad data. All worked as expected.

Just a thought:

Would it be useful to log at the error-level any instances that weren't properly converted over, e.g.

  LOG.error("Instance %s did not have instance_type_id converted to an int because its value is" % (instance.id, instance.instance_type_id))

review: Approve

[Dan Prince (dan-prince)]

So hey Mark. Kind of unfortunate Postgres doesn't cast this automatically within the alter table command. I just tinkered with a simple Postgres table and I have to do something like this to get it to work:

  ALTER TABLE instances ALTER COLUMN instance_type_id TYPE integer USING instance_type_id::int;

Initially I just tried a simple:

  ALTER TABLE instances ALTER COLUMN instance_type_id TYPE integer;

The second command gives a casting error.

In any case it looks like what you have done is the best we can do in terms of converting data in a database agnostic fashion.

Looks good.

review: Approve

[Dan Prince (dan-prince)]

Hey Mark,

Heads up. Your quote limit branch just went in so you need to bump the migration version from 016 to 017.

review: Needs Fixing

[Mark Washenberger (markwash)]

Bumped the version and added some error logging. Should be good to go now, although if any of the other db migrations go in before this I suppose I will have to change filenames again.

[Dan Prince (dan-prince)]

Hey Mark.

Looks good now.

review: Approve

=== added file 'nova/db/sqlalchemy/migrate_repo/versions/017_make_instance_type_id_an_integer.py'
--- nova/db/sqlalchemy/migrate_repo/versions/017_make_instance_type_id_an_integer.py	1970-01-01 00:00:00 +0000
+++ nova/db/sqlalchemy/migrate_repo/versions/017_make_instance_type_id_an_integer.py	2011-05-17 16:37:31 +0000
@@ -0,0 +1,68 @@
+from sqlalchemy import Column, Integer, MetaData, String, Table
+from nova import log as logging
+
+meta = MetaData()
+
+
+def upgrade(migrate_engine):
+    meta.bind = migrate_engine
+    instances = Table('instances', meta, autoload=True,
+                      autoload_with=migrate_engine)
+
+    types = {}
+    for instance in migrate_engine.execute(instances.select()):
+        if instance.instance_type_id is None:
+            types[instance.id] = None
+            continue
+        try:
+            types[instance.id] = int(instance.instance_type_id)
+        except ValueError:
+            logging.warn("Instance %s did not have instance_type_id "
+                         "converted to an integer because its value is %s" %
+                          (instance.id, instance.instance_type_id))
+            types[instance.id] = None
+
+    integer_column = Column('instance_type_id_int', Integer(), nullable=True)
+    string_column = instances.c.instance_type_id
+
+    integer_column.create(instances)
+    for instance_id, instance_type_id in types.iteritems():
+        update = instances.update().\
+                where(instances.c.id == instance_id).\
+                values(instance_type_id_int=instance_type_id)
+        migrate_engine.execute(update)
+
+    string_column.alter(name='instance_type_id_str')
+    integer_column.alter(name='instance_type_id')
+    string_column.drop()
+
+
+def downgrade(migrate_engine):
+    meta.bind = migrate_engine
+    instances = Table('instances', meta, autoload=True,
+                      autoload_with=migrate_engine)
+
+    integer_column = instances.c.instance_type_id
+    string_column = Column('instance_type_id_str',
+                           String(length=255, convert_unicode=False,
+                                  assert_unicode=None, unicode_error=None,
+                                  _warn_on_bytestring=False),
+                           nullable=True)
+
+    types = {}
+    for instance in migrate_engine.execute(instances.select()):
+        if instance.instance_type_id is None:
+            types[instance.id] = None
+        else:
+            types[instance.id] = str(instance.instance_type_id)
+
+    string_column.create(instances)
+    for instance_id, instance_type_id in types.iteritems():
+        update = instances.update().\
+                where(instances.c.id == instance_id).\
+                values(instance_type_id_str=instance_type_id)
+        migrate_engine.execute(update)
+
+    integer_column.alter(name='instance_type_id_int')
+    string_column.alter(name='instance_type_id')
+    integer_column.drop()

=== modified file 'nova/db/sqlalchemy/models.py'
--- nova/db/sqlalchemy/models.py	2011-05-12 06:27:47 +0000
+++ nova/db/sqlalchemy/models.py	2011-05-17 16:37:31 +0000
@@ -209,7 +209,7 @@
     hostname = Column(String(255))
     host = Column(String(255))  # , ForeignKey('hosts.id'))
 
-    instance_type_id = Column(String(255))
+    instance_type_id = Column(Integer)
 
     user_data = Column(Text)