[openstack-dev] savann-ci, Re: [savanna] Alembic migrations and absence of DROP column in sqlite
Sergey Lukjanov
slukjanov at mirantis.com
Wed Feb 5 19:21:21 UTC 2014
Trevor, I've created an issue to track it
https://bugs.launchpad.net/savanna/+bug/1276764
On Wed, Feb 5, 2014 at 8:56 PM, Trevor McKay <tmckay at redhat.com> wrote:
> Hi Sergey,
>
> Is there a bug or a blueprint for this? I did a quick search but
> didn't see one.
>
> Thanks,
>
> Trevor
>
> On Wed, 2014-02-05 at 16:06 +0400, Sergey Kolekonov wrote:
> > I'm currently working on moving on the MySQL for savanna-ci
> >
> >
> > On Wed, Feb 5, 2014 at 3:53 PM, Sergey Lukjanov
> > <slukjanov at mirantis.com> wrote:
> > Agreed, let's move on to the MySQL for savanna-ci to run
> > integration tests against production-like DB.
> >
> >
> > On Wed, Feb 5, 2014 at 1:54 AM, Andrew Lazarev
> > <alazarev at mirantis.com> wrote:
> > Since sqlite is not in the list of "databases that
> > would be used in production", CI should use other DB
> > for testing.
> >
> >
> > Andrew.
> >
> >
> > On Tue, Feb 4, 2014 at 1:13 PM, Alexander Ignatov
> > <aignatov at mirantis.com> wrote:
> > 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
> > <tmckay at 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
> > >> <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
> > >
> > >
> > >
> > >
> > _______________________________________________
> > > 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
> >
> >
> >
> >
> >
> > --
> > Sincerely yours,
> > Sergey Lukjanov
> > Savanna Technical Lead
> > Mirantis Inc.
> >
> > _______________________________________________
> > 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
>
--
Sincerely yours,
Sergey Lukjanov
Savanna Technical Lead
Mirantis Inc.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20140205/10b78112/attachment.html>
More information about the OpenStack-dev
mailing list