Open Stack

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