<div dir="ltr">Since sqlite is not in the list of "<span style="font-size:13px;font-family:arial,sans-serif">databases that would be used in production", </span>CI should use other DB for testing.<br><div><br></div>
<div>Andrew.</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Feb 4, 2014 at 1:13 PM, Alexander Ignatov <span dir="ltr"><<a href="mailto:aignatov@mirantis.com" target="_blank">aignatov@mirantis.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Indeed. We should create a bug around that and move our savanna-ci to mysql.<br>
<br>
Regards,<br>
Alexander Ignatov<br>
<br>
<br>
<br>
On 05 Feb 2014, at 01:01, Trevor McKay <<a href="mailto:tmckay@redhat.com">tmckay@redhat.com</a>> wrote:<br>
<br>
> This brings up an interesting problem:<br>
><br>
> In <a href="https://review.openstack.org/#/c/70420/" target="_blank">https://review.openstack.org/#/c/70420/</a> I've added a migration that<br>
> uses a drop column for an upgrade.<br>
><br>
> But savann-ci is apparently using a sqlite database to run. So it can't<br>
> possibly pass.<br>
><br>
> What do we do here? Shift savanna-ci tests to non sqlite?<br>
><br>
> Trevor<br>
><br>
> On Sat, 2014-02-01 at 18:17 +0200, Roman Podoliaka wrote:<br>
>> Hi all,<br>
>><br>
>> My two cents.<br>
>><br>
>>> 2) Extend alembic so that op.drop_column() does the right thing<br>
>> We could, but should we?<br>
>><br>
>> The only reason alembic doesn't support these operations for SQLite<br>
>> yet is that SQLite lacks proper support of ALTER statement. For<br>
>> sqlalchemy-migrate we've been providing a work-around in the form of<br>
>> recreating of a table and copying of all existing rows (which is a<br>
>> hack, really).<br>
>><br>
>> But to be able to recreate a table, we first must have its definition.<br>
>> And we've been relying on SQLAlchemy schema reflection facilities for<br>
>> that. Unfortunately, this approach has a few drawbacks:<br>
>><br>
>> 1) SQLAlchemy versions prior to 0.8.4 don't support reflection of<br>
>> unique constraints, which means the recreated table won't have them;<br>
>><br>
>> 2) special care must be taken in 'edge' cases (e.g. when you want to<br>
>> drop a BOOLEAN column, you must also drop the corresponding CHECK (col<br>
>> in (0, 1)) constraint manually, or SQLite will raise an error when the<br>
>> table is recreated without the column being dropped)<br>
>><br>
>> 3) special care must be taken for 'custom' type columns (it's got<br>
>> better with SQLAlchemy 0.8.x, but e.g. in 0.7.x we had to override<br>
>> definitions of reflected BIGINT columns manually for each<br>
>> column.drop() call)<br>
>><br>
>> 4) schema reflection can't be performed when alembic migrations are<br>
>> run in 'offline' mode (without connecting to a DB)<br>
>> ...<br>
>> (probably something else I've forgotten)<br>
>><br>
>> So it's totally doable, but, IMO, there is no real benefit in<br>
>> supporting running of schema migrations for SQLite.<br>
>><br>
>>> ...attempts to drop schema generation based on models in favor of migrations<br>
>><br>
>> As long as we have a test that checks that the DB schema obtained by<br>
>> running of migration scripts is equal to the one obtained by calling<br>
>> metadata.create_all(), it's perfectly OK to use model definitions to<br>
>> generate the initial DB schema for running of unit-tests as well as<br>
>> for new installations of OpenStack (and this is actually faster than<br>
>> running of migration scripts). ... and if we have strong objections<br>
>> against doing metadata.create_all(), we can always use migration<br>
>> scripts for both new installations and upgrades for all DB backends,<br>
>> except SQLite.<br>
>><br>
>> Thanks,<br>
>> Roman<br>
>><br>
>> On Sat, Feb 1, 2014 at 12:09 PM, Eugene Nikanorov<br>
>> <<a href="mailto:enikanorov@mirantis.com">enikanorov@mirantis.com</a>> wrote:<br>
>>> Boris,<br>
>>><br>
>>> Sorry for the offtopic.<br>
>>> Is switching to model-based schema generation is something decided? I see<br>
>>> the opposite: attempts to drop schema generation based on models in favor of<br>
>>> migrations.<br>
>>> Can you point to some discussion threads?<br>
>>><br>
>>> Thanks,<br>
>>> Eugene.<br>
>>><br>
>>><br>
>>><br>
>>> On Sat, Feb 1, 2014 at 2:19 AM, Boris Pavlovic <<a href="mailto:bpavlovic@mirantis.com">bpavlovic@mirantis.com</a>><br>
>>> wrote:<br>
>>>><br>
>>>> Jay,<br>
>>>><br>
>>>> Yep we shouldn't use migrations for sqlite at all.<br>
>>>><br>
>>>> The major issue that we have now is that we are not able to ensure that DB<br>
>>>> schema created by migration & models are same (actually they are not same).<br>
>>>><br>
>>>> So before dropping support of migrations for sqlite & switching to model<br>
>>>> based created schema we should add tests that will check that model &<br>
>>>> migrations are synced.<br>
>>>> (we are working on this)<br>
>>>><br>
>>>><br>
>>>><br>
>>>> Best regards,<br>
>>>> Boris Pavlovic<br>
>>>><br>
>>>><br>
>>>> On Fri, Jan 31, 2014 at 7:31 PM, Andrew Lazarev <<a href="mailto:alazarev@mirantis.com">alazarev@mirantis.com</a>><br>
>>>> wrote:<br>
>>>>><br>
>>>>> Trevor,<br>
>>>>><br>
>>>>> Such check could be useful on alembic side too. Good opportunity for<br>
>>>>> contribution.<br>
>>>>><br>
>>>>> Andrew.<br>
>>>>><br>
>>>>><br>
>>>>> On Fri, Jan 31, 2014 at 6:12 AM, Trevor McKay <<a href="mailto:tmckay@redhat.com">tmckay@redhat.com</a>> wrote:<br>
>>>>>><br>
>>>>>> Okay, I can accept that migrations shouldn't be supported on sqlite.<br>
>>>>>><br>
>>>>>> However, if that's the case then we need to fix up savanna-db-manage so<br>
>>>>>> that it checks the db connection info and throws a polite error to the<br>
>>>>>> user for attempted migrations on unsupported platforms. For example:<br>
>>>>>><br>
>>>>>> "Database migrations are not supported for sqlite"<br>
>>>>>><br>
>>>>>> Because, as a developer, when I see a sql error trace as the result of<br>
>>>>>> an operation I assume it's broken :)<br>
>>>>>><br>
>>>>>> Best,<br>
>>>>>><br>
>>>>>> Trevor<br>
>>>>>><br>
>>>>>> On Thu, 2014-01-30 at 15:04 -0500, Jay Pipes wrote:<br>
>>>>>>> On Thu, 2014-01-30 at 14:51 -0500, Trevor McKay wrote:<br>
>>>>>>>> I was playing with alembic migration and discovered that<br>
>>>>>>>> op.drop_column() doesn't work with sqlite. This is because sqlite<br>
>>>>>>>> doesn't support dropping a column (broken imho, but that's another<br>
>>>>>>>> discussion). Sqlite throws a syntax error.<br>
>>>>>>>><br>
>>>>>>>> To make this work with sqlite, you have to copy the table to a<br>
>>>>>>>> temporary<br>
>>>>>>>> excluding the column(s) you don't want and delete the old one,<br>
>>>>>>>> followed<br>
>>>>>>>> by a rename of the new table.<br>
>>>>>>>><br>
>>>>>>>> The existing 002 migration uses op.drop_column(), so I'm assuming<br>
>>>>>>>> it's<br>
>>>>>>>> broken, too (I need to check what the migration test is doing). I<br>
>>>>>>>> was<br>
>>>>>>>> working on an 003.<br>
>>>>>>>><br>
>>>>>>>> How do we want to handle this? Three good options I can think of:<br>
>>>>>>>><br>
>>>>>>>> 1) don't support migrations for sqlite (I think "no", but maybe)<br>
>>>>>>>><br>
>>>>>>>> 2) Extend alembic so that op.drop_column() does the right thing<br>
>>>>>>>> (more<br>
>>>>>>>> open-source contributions for us, yay :) )<br>
>>>>>>>><br>
>>>>>>>> 3) Add our own wrapper in savanna so that we have a drop_column()<br>
>>>>>>>> method<br>
>>>>>>>> that wraps copy/rename.<br>
>>>>>>>><br>
>>>>>>>> Ideas, comments?<br>
>>>>>>><br>
>>>>>>> Migrations should really not be run against SQLite at all -- only on<br>
>>>>>>> the<br>
>>>>>>> databases that would be used in production. I believe the general<br>
>>>>>>> direction of the contributor community is to be consistent around<br>
>>>>>>> testing of migrations and to not run migrations at all in unit tests<br>
>>>>>>> (which use SQLite).<br>
>>>>>>><br>
>>>>>>> Boris (cc'd) may have some more to say on this topic.<br>
>>>>>>><br>
>>>>>>> Best,<br>
>>>>>>> -jay<br>
>>>>>>><br>
>>>>>>><br>
>>>>>>> _______________________________________________<br>
>>>>>>> OpenStack-dev mailing list<br>
>>>>>>> <a href="mailto:OpenStack-dev@lists.openstack.org">OpenStack-dev@lists.openstack.org</a><br>
>>>>>>> <a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
>>>>>><br>
>>>>>><br>
>>>>>><br>
>>>>>> _______________________________________________<br>
>>>>>> OpenStack-dev mailing list<br>
>>>>>> <a href="mailto:OpenStack-dev@lists.openstack.org">OpenStack-dev@lists.openstack.org</a><br>
>>>>>> <a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
>>>>><br>
>>>>><br>
>>>><br>
>>>><br>
>>>> _______________________________________________<br>
>>>> OpenStack-dev mailing list<br>
>>>> <a href="mailto:OpenStack-dev@lists.openstack.org">OpenStack-dev@lists.openstack.org</a><br>
>>>> <a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
>>>><br>
>>><br>
>>><br>
>>> _______________________________________________<br>
>>> OpenStack-dev mailing list<br>
>>> <a href="mailto:OpenStack-dev@lists.openstack.org">OpenStack-dev@lists.openstack.org</a><br>
>>> <a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
>>><br>
>><br>
>> _______________________________________________<br>
>> OpenStack-dev mailing list<br>
>> <a href="mailto:OpenStack-dev@lists.openstack.org">OpenStack-dev@lists.openstack.org</a><br>
>> <a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
><br>
><br>
><br>
> _______________________________________________<br>
> OpenStack-dev mailing list<br>
> <a href="mailto:OpenStack-dev@lists.openstack.org">OpenStack-dev@lists.openstack.org</a><br>
> <a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
<br>
<br>
_______________________________________________<br>
OpenStack-dev mailing list<br>
<a href="mailto:OpenStack-dev@lists.openstack.org">OpenStack-dev@lists.openstack.org</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
</blockquote></div><br></div>