Open Stack

Tue Feb 4 21:13:56 UTC 2014

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

Open Stack

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

OpenStack

Community

Documentation

Branding & Legal