FYI Introducing oslo.service 1.31.7 with the latest merged changes. These refers to #3. https://review.openstack.org/#/c/620913/ Le mer. 28 nov. 2018 à 00:31, Ben Nemec <openstack@nemebean.com> a écrit :
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
-- Hervé Beraud Senior Software Engineer Red Hat - Openstack Oslo irc: hberaud -----BEGIN PGP SIGNATURE----- wsFcBAABCAAQBQJb4AwCCRAHwXRBNkGNegAALSkQAHrotwCiL3VMwDR0vcja10Q+ Kf31yCutl5bAlS7tOKpPQ9XN4oC0ZSThyNNFVrg8ail0SczHXsC4rOrsPblgGRN+ RQLoCm2eO1AkB0ubCYLaq0XqSaO+Uk81QxAPkyPCEGT6SRxXr2lhADK0T86kBnMP F8RvGolu3EFjlqCVgeOZaR51PqwUlEhZXZuuNKrWZXg/oRiY4811GmnvzmUhgK5G 5+f8mUg74hfjDbR2VhjTeaLKp0PhskjOIKY3vqHXofLuaqFDD+WrAy/NgDGvN22g glGfj472T3xyHnUzM8ILgAGSghfzZF5Skj2qEeci9cB6K3Hm3osj+PbvfsXE/7Kw m/xtm+FjnaywZEv54uCmVIzQsRIm1qJscu20Qw6Q0UiPpDFqD7O6tWSRKdX11UTZ hwVQTMh9AKQDBEh2W9nnFi9kzSSNu4OQ1dRMcYHWfd9BEkccezxHwUM4Xyov5Fe0 qnbfzTB1tYkjU78loMWFaLa00ftSxP/DtQ//iYVyfVNfcCwfDszXLOqlkvGmY1/Y F1ON0ONekDZkGJsDoS6QdiUSn8RZ2mHArGEWMV00EV5DCIbCXRvywXV43ckx8Z+3 B8qUJhBqJ8RS2F+vTs3DTaXqcktgJ4UkhYC2c1gImcPRyGrK9VY0sCT+1iA+wp/O v6rDpkeNksZ9fFSyoY2o =ECSj -----END PGP SIGNATURE-----