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

Sergey Lukjanov slukjanov at mirantis.com
Wed Feb 5 14:22:06 UTC 2014


It's about integration tests that aren't db-specific, so, just
DATABASE/connection should be fixed ;)


On Wed, Feb 5, 2014 at 4:33 PM, Alexei Kornienko <alexei.kornienko at gmail.com
> wrote:

>  Hi
>
>
> I'm currently working on moving on the MySQL for savanna-ci
>
> We are working on same task in ceilometer so maybe you could use some of
> our patches as reference:
>
> https://review.openstack.org/#/c/59489/
> https://review.openstack.org/#/c/63049/
>
> Regards,
> Alexei
>
>
> On 02/05/2014 02:06 PM, 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 listOpenStack-dev at lists.openstack.orghttp://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/76e3d2c2/attachment.html>


More information about the OpenStack-dev mailing list