Indeed. We should create a bug around that and move our savanna-ci to mysql.
Regards, Alexander Ignatov On 05 Feb 2014, at 01:01, Trevor McKay <tmc...@redhat.com> wrote: > This brings up an interesting problem: > > In https://review.openstack.org/#/c/70420/ I've added a migration that > uses a drop column for an upgrade. > > But savann-ci is apparently using a sqlite database to run. So it can't > possibly pass. > > What do we do here? Shift savanna-ci tests to non sqlite? > > Trevor > > On Sat, 2014-02-01 at 18:17 +0200, Roman Podoliaka wrote: >> 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 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 >>> >> >> _______________________________________________ >> 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