Re: [openstack-dev] [savanna] Alembic migrations and absence of DROP column in sqlite
Boris, Sorry for the offtopic. Is switching to model-based schema generation is something decided? I see the opposite: attempts to drop schema generation based on models in favor of migrations. Can you point to some discussion threads? Thanks, Eugene. On Sat, Feb 1, 2014 at 2:19 AM, Boris Pavlovic bpavlo...@mirantis.comwrote: Jay, Yep we shouldn't use migrations for sqlite at all. The major issue that we have now is that we are not able to ensure that DB schema created by migration models are same (actually they are not same). So before dropping support of migrations for sqlite switching to model based created schema we should add tests that will check that model migrations are synced. (we are working on this) Best regards, Boris Pavlovic On Fri, Jan 31, 2014 at 7:31 PM, Andrew Lazarev alaza...@mirantis.comwrote: Trevor, Such check could be useful on alembic side too. Good opportunity for contribution. Andrew. On Fri, Jan 31, 2014 at 6:12 AM, Trevor McKay tmc...@redhat.com wrote: Okay, I can accept that migrations shouldn't be supported on sqlite. However, if that's the case then we need to fix up savanna-db-manage so that it checks the db connection info and throws a polite error to the user for attempted migrations on unsupported platforms. For example: Database migrations are not supported for sqlite Because, as a developer, when I see a sql error trace as the result of an operation I assume it's broken :) Best, Trevor On Thu, 2014-01-30 at 15:04 -0500, Jay Pipes wrote: On Thu, 2014-01-30 at 14:51 -0500, Trevor McKay wrote: I was playing with alembic migration and discovered that op.drop_column() doesn't work with sqlite. This is because sqlite doesn't support dropping a column (broken imho, but that's another discussion). Sqlite throws a syntax error. To make this work with sqlite, you have to copy the table to a temporary excluding the column(s) you don't want and delete the old one, followed by a rename of the new table. The existing 002 migration uses op.drop_column(), so I'm assuming it's broken, too (I need to check what the migration test is doing). I was working on an 003. How do we want to handle this? Three good options I can think of: 1) don't support migrations for sqlite (I think no, but maybe) 2) Extend alembic so that op.drop_column() does the right thing (more open-source contributions for us, yay :) ) 3) Add our own wrapper in savanna so that we have a drop_column() method that wraps copy/rename. Ideas, comments? Migrations should really not be run against SQLite at all -- only on the databases that would be used in production. I believe the general direction of the contributor community is to be consistent around testing of migrations and to not run migrations at all in unit tests (which use SQLite). Boris (cc'd) may have some more to say on this topic. Best, -jay ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [savanna] Alembic migrations and absence of DROP column in sqlite
Hi all, My two cents. 2) Extend alembic so that op.drop_column() does the right thing We could, but should we? The only reason alembic doesn't support these operations for SQLite yet is that SQLite lacks proper support of ALTER statement. For sqlalchemy-migrate we've been providing a work-around in the form of recreating of a table and copying of all existing rows (which is a hack, really). But to be able to recreate a table, we first must have its definition. And we've been relying on SQLAlchemy schema reflection facilities for that. Unfortunately, this approach has a few drawbacks: 1) SQLAlchemy versions prior to 0.8.4 don't support reflection of unique constraints, which means the recreated table won't have them; 2) special care must be taken in 'edge' cases (e.g. when you want to drop a BOOLEAN column, you must also drop the corresponding CHECK (col in (0, 1)) constraint manually, or SQLite will raise an error when the table is recreated without the column being dropped) 3) special care must be taken for 'custom' type columns (it's got better with SQLAlchemy 0.8.x, but e.g. in 0.7.x we had to override definitions of reflected BIGINT columns manually for each column.drop() call) 4) schema reflection can't be performed when alembic migrations are run in 'offline' mode (without connecting to a DB) ... (probably something else I've forgotten) So it's totally doable, but, IMO, there is no real benefit in supporting running of schema migrations for SQLite. ...attempts to drop schema generation based on models in favor of migrations As long as we have a test that checks that the DB schema obtained by running of migration scripts is equal to the one obtained by calling metadata.create_all(), it's perfectly OK to use model definitions to generate the initial DB schema for running of unit-tests as well as for new installations of OpenStack (and this is actually faster than running of migration scripts). ... and if we have strong objections against doing metadata.create_all(), we can always use migration scripts for both new installations and upgrades for all DB backends, except SQLite. Thanks, Roman On Sat, Feb 1, 2014 at 12:09 PM, Eugene Nikanorov enikano...@mirantis.com wrote: Boris, Sorry for the offtopic. Is switching to model-based schema generation is something decided? I see the opposite: attempts to drop schema generation based on models in favor of migrations. Can you point to some discussion threads? Thanks, Eugene. On Sat, Feb 1, 2014 at 2:19 AM, Boris Pavlovic bpavlo...@mirantis.com wrote: Jay, Yep we shouldn't use migrations for sqlite at all. The major issue that we have now is that we are not able to ensure that DB schema created by migration models are same (actually they are not same). So before dropping support of migrations for sqlite switching to model based created schema we should add tests that will check that model migrations are synced. (we are working on this) Best regards, Boris Pavlovic On Fri, Jan 31, 2014 at 7:31 PM, Andrew Lazarev alaza...@mirantis.com wrote: Trevor, Such check could be useful on alembic side too. Good opportunity for contribution. Andrew. On Fri, Jan 31, 2014 at 6:12 AM, Trevor McKay tmc...@redhat.com wrote: Okay, I can accept that migrations shouldn't be supported on sqlite. However, if that's the case then we need to fix up savanna-db-manage so that it checks the db connection info and throws a polite error to the user for attempted migrations on unsupported platforms. For example: Database migrations are not supported for sqlite Because, as a developer, when I see a sql error trace as the result of an operation I assume it's broken :) Best, Trevor On Thu, 2014-01-30 at 15:04 -0500, Jay Pipes wrote: On Thu, 2014-01-30 at 14:51 -0500, Trevor McKay wrote: I was playing with alembic migration and discovered that op.drop_column() doesn't work with sqlite. This is because sqlite doesn't support dropping a column (broken imho, but that's another discussion). Sqlite throws a syntax error. To make this work with sqlite, you have to copy the table to a temporary excluding the column(s) you don't want and delete the old one, followed by a rename of the new table. The existing 002 migration uses op.drop_column(), so I'm assuming it's broken, too (I need to check what the migration test is doing). I was working on an 003. How do we want to handle this? Three good options I can think of: 1) don't support migrations for sqlite (I think no, but maybe) 2) Extend alembic so that op.drop_column() does the right thing (more open-source contributions for us, yay :) ) 3) Add our own wrapper in savanna so that we have a drop_column() method that wraps copy/rename. Ideas, comments? Migrations should really not be run against SQLite at all -- only on the databases that would be used in production. I
Re: [openstack-dev] [savanna] Alembic migrations and absence of DROP column in sqlite
Jay, Yep we shouldn't use migrations for sqlite at all. The major issue that we have now is that we are not able to ensure that DB schema created by migration models are same (actually they are not same). So before dropping support of migrations for sqlite switching to model based created schema we should add tests that will check that model migrations are synced. (we are working on this) Best regards, Boris Pavlovic On Fri, Jan 31, 2014 at 7:31 PM, Andrew Lazarev alaza...@mirantis.comwrote: Trevor, Such check could be useful on alembic side too. Good opportunity for contribution. Andrew. On Fri, Jan 31, 2014 at 6:12 AM, Trevor McKay tmc...@redhat.com wrote: Okay, I can accept that migrations shouldn't be supported on sqlite. However, if that's the case then we need to fix up savanna-db-manage so that it checks the db connection info and throws a polite error to the user for attempted migrations on unsupported platforms. For example: Database migrations are not supported for sqlite Because, as a developer, when I see a sql error trace as the result of an operation I assume it's broken :) Best, Trevor On Thu, 2014-01-30 at 15:04 -0500, Jay Pipes wrote: On Thu, 2014-01-30 at 14:51 -0500, Trevor McKay wrote: I was playing with alembic migration and discovered that op.drop_column() doesn't work with sqlite. This is because sqlite doesn't support dropping a column (broken imho, but that's another discussion). Sqlite throws a syntax error. To make this work with sqlite, you have to copy the table to a temporary excluding the column(s) you don't want and delete the old one, followed by a rename of the new table. The existing 002 migration uses op.drop_column(), so I'm assuming it's broken, too (I need to check what the migration test is doing). I was working on an 003. How do we want to handle this? Three good options I can think of: 1) don't support migrations for sqlite (I think no, but maybe) 2) Extend alembic so that op.drop_column() does the right thing (more open-source contributions for us, yay :) ) 3) Add our own wrapper in savanna so that we have a drop_column() method that wraps copy/rename. Ideas, comments? Migrations should really not be run against SQLite at all -- only on the databases that would be used in production. I believe the general direction of the contributor community is to be consistent around testing of migrations and to not run migrations at all in unit tests (which use SQLite). Boris (cc'd) may have some more to say on this topic. Best, -jay ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
[openstack-dev] [savanna] Alembic migrations and absence of DROP column in sqlite
I was playing with alembic migration and discovered that op.drop_column() doesn't work with sqlite. This is because sqlite doesn't support dropping a column (broken imho, but that's another discussion). Sqlite throws a syntax error. To make this work with sqlite, you have to copy the table to a temporary excluding the column(s) you don't want and delete the old one, followed by a rename of the new table. The existing 002 migration uses op.drop_column(), so I'm assuming it's broken, too (I need to check what the migration test is doing). I was working on an 003. How do we want to handle this? Three good options I can think of: 1) don't support migrations for sqlite (I think no, but maybe) 2) Extend alembic so that op.drop_column() does the right thing (more open-source contributions for us, yay :) ) 3) Add our own wrapper in savanna so that we have a drop_column() method that wraps copy/rename. Ideas, comments? Best, Trevor ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [savanna] Alembic migrations and absence of DROP column in sqlite
On Thu, 2014-01-30 at 14:51 -0500, Trevor McKay wrote: I was playing with alembic migration and discovered that op.drop_column() doesn't work with sqlite. This is because sqlite doesn't support dropping a column (broken imho, but that's another discussion). Sqlite throws a syntax error. To make this work with sqlite, you have to copy the table to a temporary excluding the column(s) you don't want and delete the old one, followed by a rename of the new table. The existing 002 migration uses op.drop_column(), so I'm assuming it's broken, too (I need to check what the migration test is doing). I was working on an 003. How do we want to handle this? Three good options I can think of: 1) don't support migrations for sqlite (I think no, but maybe) 2) Extend alembic so that op.drop_column() does the right thing (more open-source contributions for us, yay :) ) 3) Add our own wrapper in savanna so that we have a drop_column() method that wraps copy/rename. Ideas, comments? Migrations should really not be run against SQLite at all -- only on the databases that would be used in production. I believe the general direction of the contributor community is to be consistent around testing of migrations and to not run migrations at all in unit tests (which use SQLite). Boris (cc'd) may have some more to say on this topic. Best, -jay ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
