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