<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Jul 3, 2013 at 6:50 AM, Michael Still <span dir="ltr"><<a href="mailto:mikal@stillhq.com" target="_blank">mikal@stillhq.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class="im">On Wed, Jul 3, 2013 at 3:50 AM, Boris Pavlovic <<a href="mailto:boris@pavlovic.me">boris@pavlovic.me</a>> wrote:<br>
<br>
> Question:<br>
> Why we should put in oslo slqlalchemy-migrate monkey patches, when we are<br>
> planing to switch to alembic?<br>
><br>
> Answer:<br>
> If we don’t put in oslo sqlalchemy-migrate monkey patches. We won't be<br>
> able to work on 7 point at all until 8 and 10 points will be implemented in<br>
> every project. Also work around 8 point is not finished, so we are not able<br>
> to implement 10 points in any of project. So this blocks almost all work in<br>
> all projects. I think that these 100-200 lines of code are not so big price<br>
> for saving few cycles of time.<br>
<br>
</div>We've talked in the past (Folsom summit?) about alembic, but I'm not<br>
aware of anyone who is actually working on it. Is someone working on<br>
moving us to alembic? If not, it seems unfair to block database work<br>
on something no one is actually working on.<br></blockquote><div><br></div><div style>That's not quite what happened. Unfortunately the conversation happened in gerrit, IRC, and email, so it's a little hard to piece together from the outside.</div>
<div style><br></div><div style>I had several concerns with the nature of this change, not the least of which is it is monkey-patching a third-party library to add a feature instead of just modifying that library upstream.</div>
<div style><div><br></div><div>The patch I objected to (<a href="https://review.openstack.org/#/c/31016">https://review.openstack.org/#/c/31016</a>) modifies the sqlite driver inside sqlalchemy-migrate to support some migration patterns that it does not support natively. There's no blueprint linked from the commit message on the patch I was reviewing, so I didn't have the full background. The description of the patch, and the discussion in gerrit, initially led me to believe this was for unit tests for the migrations themselves. I pointed out that it didn't make any sense to test the migrations on a database no one would use in production, especially if we had to monkey patch the driver to make the migrations work in the first place.</div>
<div><br></div><div>Boris clarified that the tests were the general nova tests, at which point I asked why nova was relying on the migrations to set up a database for its tests instead of just using the models. Sean cleared up the history on that point, and although I'm still not happy with the idea of putting code in oslo with the pre-declared plan to remove it (rather than consider it for graduation), I agreed that the pragmatic thing to do for now is to live with the monkey patched version of sqlalchemy-migrate.<br>
</div><div><br></div></div><div style>At this point, I have removed my -2 to the patch, but I haven't had a chance to fully review the code. I voted 0 to unblock it in case other reviewers had time to look at it before I was able to come back. That hasn't happened, but the patch is no longer blocked.</div>
<div style><div><br class="">Somewhere during that conversation, I suggested looking at alembic as an alternative, but alembic clearly states in its documentation that migrations on sqlite are not supported because of the database's limited support for alter statements, but that if someone wants to contribute those features patches would be welcome. If we do need this feature to support good unit tests of SQLalchemy-based projects, we should eventually move it out of oslo and into alembic, then move our migration scripts to use alembic. It would make the most sense to do that on a release boundary, when we normally collapse the migration scripts anyway. Even better would be if we could make the models and migration scripts produce databases that are compatible enough for testing the main project, and then run tests for the migrations themselves against real databases as a separate step. Based on the plan Boris has posted, it sounds like he is working toward both of these goals.</div>
<div><br></div></div><div style>Doug</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<span class=""><font color="#888888"><br>
Michael<br>
</font></span><div class=""><div class="h5"><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></div></div>