Open Stack

On 11/26/18 9:47 AM, Zane Bitter wrote:
> On 22/11/18 6:43 PM, Tony Breeds wrote:
>>> ### 3 - Maintain a private interface for ThreadingEvent only on 
>>> stable/rocky
>>> impacted projects: oslo.service
>>> related reviews:
>>> -https://review.openstack.org/619342/
>>>
>>> pros:
>>> - straightforward
>>> cons:
>>> - Changing the loopingcall module semantics as it is different type
>>> - stable only patches
>>> - misunderstoud service customer between Threading, eventlet, etc.. and
>>> master behavior
>> A variation of this (without adding the debtcollector requirement) might
>> work as IIUC all versions of oslo.service < 1.32.0 (except 1.31.6) would
>> work and doesn't introduce any new policy violations.
>>
>> Adding debcollector is great but introduces at least the semver issues
>> from option #1
> 
> Hey Tony, can you explain the debtcollector issue a bit more? Is it just 
> that we generally promise to not adding anything to requirements.txt on 
> stable branches? In this case, debtcollector, appears to already be a 
> transitive dependency of something that oslo.service already requires 
> (it appears in lower-constraints.txt, at least), so I was assuming that 
> there wouldn't be any real-world consequences. Does that change things?

Technically adding a new dep requires a minor version update per our 
release guidelines. I guess it gets a little fuzzy when you're adding a 
dep that's already part of the transitive set, but I'm guessing that's 
what Tony was referring to.

That said, as I mentioned on the review I don't feel any particular need 
to debtcollector this. It's in a private module that nobody should ever 
have been using and we're only doing this because we prioritize fixing 
the bug over absolute API purity. :-)

In any case, I'm +1 on doing this to avoid creating weird version 
dependencies on a stable branch.

> 
> TBH it would be trivial to do something equivalent without adding the 
> new requirement, so I guess the easiest thing is if I just update the 
> patch to do that.
> 
>>> ### 4 - Don't use private interface in oslo.service
>>> impacted projects: nova
>>> related reviews:
>>> -https://review.openstack.org/#/c/619360/
>>>
>>> pros:
>>> - straightforward
>>> cons:
>>> - this could work but it is not the same sematics as before as the 
>>> type has
>>> changed
>>> - stable only patches
>>> - misunderstoud service customer between Threading, eventlet, etc.. and
>>> master behavior
>> I think this is more or less the same as Option #3 in terms of impact.
>> If that's right it could be acceptable.
> 
> It's slightly higher impact, because you wouldn't be able to upgrade 
> oslo.service past 1.31.5 and still run the unit tests on old versions of 
> Nova. Whereas with option #3 you could just upgrade oslo.service to 
> 1.31.7, or just upgrade Nova, and either way everything would work.
> 
> Not that we shouldn't do this *as well*, but IMHO we still need to do #3.
> 

I like doing this as well. If we need an argument as to why it's okay to 
do this stable-only patch, it would be that this is essentially a 
backport of the original Nova fix proposed before we decided to do the 
fixture. In retrospect maybe we should have gone ahead and merged that 
simple, backportable fix and done the fixture as a followup, but in 
spirit this has the same effect.

-Ben