<div dir="ltr"><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Jan 23, 2014 at 10:44 AM, Sean Dague <span dir="ltr"><<a href="mailto:sean@dague.net" target="_blank">sean@dague.net</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">On 01/23/2014 09:58 AM, Doug Hellmann wrote:<br>
><br>
><br>
><br>
> On Tue, Jan 21, 2014 at 1:28 PM, Sean Dague <<a href="mailto:sean@dague.net">sean@dague.net</a><br>
</div><div class="im">> <mailto:<a href="mailto:sean@dague.net">sean@dague.net</a>>> wrote:<br>
><br>
>     On 01/21/2014 01:14 PM, Joe Gordon wrote:<br>
>     ><br>
>     > On Jan 17, 2014 12:24 AM, "Flavio Percoco" <<a href="mailto:flavio@redhat.com">flavio@redhat.com</a><br>
>     <mailto:<a href="mailto:flavio@redhat.com">flavio@redhat.com</a>><br>
</div><div class="im">>     > <mailto:<a href="mailto:flavio@redhat.com">flavio@redhat.com</a> <mailto:<a href="mailto:flavio@redhat.com">flavio@redhat.com</a>>>> wrote:<br>
>     >><br>
>     >> On 16/01/14 17:32 -0500, Doug Hellmann wrote:<br>
>     >>><br>
>     >>> On Thu, Jan 16, 2014 at 3:19 PM, Ben Nemec<br>
>     <<a href="mailto:openstack@nemebean.com">openstack@nemebean.com</a> <mailto:<a href="mailto:openstack@nemebean.com">openstack@nemebean.com</a>><br>
</div>>     > <mailto:<a href="mailto:openstack@nemebean.com">openstack@nemebean.com</a> <mailto:<a href="mailto:openstack@nemebean.com">openstack@nemebean.com</a>>>><br>
<div><div class="h5">>     wrote:<br>
>     >>><br>
>     >>>    On 2014-01-16 13:48, John Griffith wrote:<br>
>     >>><br>
>     >>>        Hey Everyone,<br>
>     >>><br>
>     >>>        A review came up today that cherry-picked a specific<br>
>     commit to<br>
>     > OSLO<br>
>     >>>        Incubator, without updating the rest of the files in the<br>
>     > module.  I<br>
>     >>>        rejected that patch, because my philosophy has been that<br>
>     when you<br>
>     >>>        update/pull from oslo-incubator it should be done as a full<br>
>     > sync of<br>
>     >>>        the entire module, not a cherry pick of the bits and pieces<br>
>     > that you<br>
>     >>>        may or may not be interested in.<br>
>     >>><br>
>     >>>        As it turns out I've received a bit of push back on this, so<br>
>     > it seems<br>
>     >>>        maybe I'm being unreasonable, or that I'm mistaken in my<br>
>     > understanding<br>
>     >>>        of the process here.  To me it seems like a complete and<br>
>     total<br>
>     > waste<br>
>     >>>        to have an oslo-incubator and common libs if you're going<br>
>     to turn<br>
>     >>>        around and just cherry pick changes, but maybe I'm completely<br>
>     > out of<br>
>     >>>        line.<br>
>     >>><br>
>     >>>        Thoughts??<br>
>     >>><br>
>     >>><br>
>     >>>    I suppose there might be exceptions, but in general I'm with you.<br>
>     >  For one<br>
>     >>>    thing, if someone tries to pull out a specific change in the Oslo<br>
>     > code,<br>
>     >>>    there's no guarantee that code even works.  Depending on how the<br>
>     > sync was<br>
>     >>>    done it's possible the code they're syncing never passed the<br>
>     Oslo unit<br>
>     >>>    tests in the form being synced, and since unit tests aren't<br>
>     synced<br>
>     > to the<br>
>     >>>    target projects it's conceivable that completely broken code<br>
>     could get<br>
>     >>>    through Jenkins.<br>
>     >>><br>
>     >>>    Obviously it's possible to do a successful partial sync, but for<br>
>     > the sake<br>
>     >>>    of reviewer sanity I'm -1 on partial syncs without a _very_ good<br>
>     > reason<br>
>     >>>    (like it's blocking the gate and there's some reason the full<br>
>     > module can't<br>
>     >>>    be synced).<br>
>     >>><br>
>     >>><br>
>     >>> I agree. Cherry picking a single (or even partial) commit really<br>
>     > should be<br>
>     >>> avoided.<br>
>     >>><br>
>     >>> The update tool does allow syncing just a single module, but that<br>
>     > should be<br>
>     >>> used very VERY carefully, especially because some of the changes<br>
>     > we're making<br>
>     >>> as we work on graduating some more libraries will include<br>
>     cross-dependent<br>
>     >>> changes between oslo modules.<br>
>     >><br>
>     >><br>
>     >> Agrred. Syncing on master should be complete synchornization from<br>
>     Oslo<br>
>     >> incubator. IMHO, the only case where cherry-picking from oslo should<br>
>     >> be allowed is when backporting patches to stable branches. Master<br>
>     >> branches should try to keep up-to-date with Oslo and sync everything<br>
>     >> every time.<br>
>     ><br>
>     > When we started Oslo incubator, we treated that code as trusted. But<br>
>     > since then there have been occasional issues when syncing the code. So<br>
>     > Oslo incubator code has lost *my* trust. Therefore I am always a<br>
>     > hesitant to do a full Oslo sync because I am not an expert on the Oslo<br>
>     > code and I risk breaking something when doing it (the issue may not<br>
>     > appear 100% of the time too). Syncing code in becomes the first time<br>
>     > that code is run against tempest, which scares me.<br>
>     ><br>
>     > I would like to propose having a integration test job in Oslo<br>
>     incubator<br>
>     > that syncs in the code, similar to how we do global requirements.<br>
><br>
>     That would be great, especially given that oslo currently doesn't run<br>
>     it's own unit tests in parallel, which means it's a lot easier for non<br>
>     parallel safe code to get in, and we don't bring the unit tests over to<br>
>     the projects on sync.<br>
><br>
>     Getting preemptive and doing a forward sync for validation would be<br>
>     really valuable.<br>
><br>
><br>
> Forcing the code in the incubator to pass the tests when synced to<br>
> projects eliminates its usefulness as a place to improve APIs. The<br>
> incubator does not make backwards compatibility claims, which is why<br>
> most of the time we want the author of code in the incubator to make the<br>
> merge and associated application code changes at the same time. When<br>
> there aren't any API changes, a straight sync should pass the tests in<br>
> the project.<br>
<br>
</div></div>So that's kind of the question at hand.<br>
<br>
If oslo-incubator is a place where we keep guarantees so that projects<br>
don't have to rework their code immediately (i.e. have a deprecation /<br>
compat period), so we can forward sync automatically, that's one thing.<br>
<br>
If oslo-incubator doesn't want to give those guarantees, then it puts<br>
more effort back on the projects, because validation on taking this code<br>
is far more manual. It also means these interfaces may very well be<br>
completely evolving in a vacuum that negatively impact the projects when<br>
synced. Which is why projects are ending 4 - 6 months behind.<br>
<br>
I think the "no guarantees" approach was fine at 5 integrated projects,<br>
only 4 of which bought onto oslo, and for far less code.<br>
<br>
At 10 integrated projects, 3 in incubation, and a requirement that new<br>
projects use oslo incubator, this juggling is getting a lot more manual,<br>
and there ends up being really large delays on these syncs.<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">Yeah, we may have outgrown the usefulness of the current approach. We're working on graduating the code to libraries, but doing so prematurely is going to make fixing some of the broken APIs (like the fact that the db library can't connect to more than one db at a time) extremely difficult, if not impossible.</div>
</div><div class="gmail_default" style="font-size:small"><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">
> We should look into why we're not running the unit tests in parallel.<br>
> That seems like something easy to change, but I'm not sure why it's like<br>
> that to begin with.<br>
<br>
</div>Because they don't pass. I looked briefly at this a couple of months<br>
ago, it wasn't an entirely straight forward fix. Again, one of the<br>
reasons why straight sync is problematic, because some part of oslo code<br>
/ tests / don't work in a parallel environment today.<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">OK, I'll open a bug about it.</div><div class="gmail_default" style="font-size:small">
<br></div><div class="gmail_default" style="font-size:small">Doug</div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5"><br>
        -Sean<br>
<br>
--<br>
Sean Dague<br>
Samsung Research America<br>
<a href="mailto:sean@dague.net">sean@dague.net</a> / <a href="mailto:sean.dague@samsung.com">sean.dague@samsung.com</a><br>
<a href="http://dague.net" target="_blank">http://dague.net</a><br>
<br>
</div></div><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></blockquote></div><br></div></div>