[openstack-dev] savann-ci, Re: [savanna] Alembic migrations and absence of DROP column in sqlite

Sergey Kolekonov skolekonov at mirantis.com
Wed Feb 5 12:06:47 UTC 2014


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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20140205/46d3fb6e/attachment.html>


More information about the OpenStack-dev mailing list