Open Stack

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
> <enikanorov at 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 <bpavlovic at 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 <alazarev at 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 <tmckay at 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 at lists.openstack.org
> >>>> > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
> >>>>
> >>>>
> >>>>
> >>>> _______________________________________________
> >>>> OpenStack-dev mailing list
> >>>> OpenStack-dev at lists.openstack.org
> >>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
> >>>
> >>>
> >>
> >>
> >> _______________________________________________
> >> OpenStack-dev mailing list
> >> OpenStack-dev at lists.openstack.org
> >> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
> >>
> >
> >
> > _______________________________________________
> > OpenStack-dev mailing list
> > OpenStack-dev at lists.openstack.org
> > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
> >
> 
> _______________________________________________
> OpenStack-dev mailing list
> OpenStack-dev at lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev