<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On 20 July 2016 at 00:06, Joshua Harlow <span dir="ltr"><<a href="mailto:harlowja@fastmail.com" target="_blank">harlowja@fastmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="">Hayes, Graham wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
On 18/07/16 22:27, Ronald Bradford wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Hi All,<br>
<br>
For Oslo libraries we ensure that API's are backward compatible for 1+ releases.<br>
When an Oslo API adds a new class attribute (as in this example of<br>
is_admin_project and 4 other attributes) added to Oslo Context in<br>
2.6.0,  these are added to ensure this API is also forward compatible<br>
with existing project code for any contract with the base class<br>
instantiation or manipulation.<br>
</blockquote>
<br>
Which projects is this run against?<br>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
The issue seen is presently Nova specific (as other projects can<br>
utilize 2.6.0) and it is related to projects that sub-class<br>
oslo.context, and how unit tests are written for using class<br>
parameters.  Ideally, to implement using oslo.context correctly<br>
OpenStack projects should:<br>
</blockquote>
<br>
Designate also had to make a quick change to support 2.6.0.<br>
<br>
We were lucky as it was noticed by the RDO builds, which had pulled in<br>
2.6.0 before the requirements update was proposed, so it did not break<br>
our gate.<br>
<br>
I just did a quick search and there is a few projects that hardcoded<br>
this, like we did.<br>
</blockquote>
<br></span>
Ya, that's bad, nothing in the docs of the to_dict API say what to even compare against (or the keys produced), so I'm pretty sure anyone doing this is setting themselves up for future failure and fragile software.<span class=""><br></span></blockquote><div><br></div><div>Can you post that list?<br> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="">
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
* Not perform direct dictionary to dictionary comparisons with the<br>
to_dict() method as this does not support when new attributes at<br>
added. Two patches (one to nova) address this in offending projects<br>
[5][6]<br>
* Unit tests should focus on attributes specific to the sub-classed<br>
signature, e.g. [7].  Oslo context provides an extensive set of unit<br>
tests for base class functionality. This is a wish list item for<br>
projects to implement.<br>
<br>
The to_dict() method exists as a convenience method only and is not an<br>
API contract. The resulting set of keys should be used accordingly.<br>
This is why there is no major release version.<br>
</blockquote>
<br>
How are developers supposed to know that?<br>
</blockquote>
<br></span>
So we (in oslo) can (and ideally will) make this better but when the API doesn't itself tell you what keys are produced or what the values of those keys are then it should be pretty obvious to u (the library user) that u can not reliably do dictionary comparisons (because how do u know what to compare against when the docs don't state that?). I suppose people are 'reverse engineering the dict' by just looking at the code but that's also not so great...<span class=""><br></span></blockquote><div><br></div><div>I think the obvious and only thing you should expect from the to_dict method is that it can be reversed by the from_dict method. Subclasses can then make small modifications to those methods to add additional information as appropriate. There is a bit of a problem in this with the way subclasses are done that is fixed in [1] but it does not affect any existing code. <br><br>We realize that the to_dict method is subclassed by a lot of services and affects RPC and so contexts must be serializable between different versions of the library so we will not modify existing to_dict values but as mentioned writing your tests to assume this will never be added to sets us up for these problems. <br><br>In this case oslo.context was largely extracted from nova and so the fragile tests make sense and should therefore be fixed - but the oslo change does not constitute a breaking API change. <br><br></div><div><br>[1] <a href="https://review.openstack.org/#/c/341250/">https://review.openstack.org/#/c/341250/</a><br> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="">
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
This kind of feels like semantics. This was an external API that changed<br>
and as a result should have been a major version.<br>
</blockquote>
<br></span>
I think this is where it gets a little bit into as u said, semantics, but the semantics IMHO are important here because it affects the ability of oslo.context to move forward & change.<br>
<br>
I suppose we should/could just put a warning on this method like I did in taskflow (for something similar) @ <a href="https://github.com/openstack/taskflow/blob/master/taskflow/engines/base.py#L71" rel="noreferrer" target="_blank">https://github.com/openstack/taskflow/blob/master/taskflow/engines/base.py#L71</a> to denote that nothing in the dict that is returned can be guaranteed to always be the same.<div class=""><div class="h5"><br>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
There is a note from our discussion in Oslo to improve our<br>
documentation to describe the API use of to_dict() better and state we<br>
will not remove to_dict() keys within a release, but that may happen<br>
between releases.<br>
<br>
There is a subsequent problem with how Nova performs a warning test<br>
[8]. Additional reviews are looking at addressing this sub-class usage<br>
of from_dict() and to_dict().<br>
<br>
Regards<br>
<br>
Ronald<br>
<br>
<br>
[5] <a href="https://review.openstack.org/#/c/343694/" rel="noreferrer" target="_blank">https://review.openstack.org/#/c/343694/</a>,<br>
[6] <a href="https://review.openstack.org/#/c/342367/" rel="noreferrer" target="_blank">https://review.openstack.org/#/c/342367/</a><br>
[7] <a href="https://review.openstack.org/#/c/342869/" rel="noreferrer" target="_blank">https://review.openstack.org/#/c/342869/</a><br>
[8] <a href="http://git.openstack.org/cgit/openstack/nova/tree/nova/tests/unit/test_context.py#n144" rel="noreferrer" target="_blank">http://git.openstack.org/cgit/openstack/nova/tree/nova/tests/unit/test_context.py#n144</a><br>
<br>
On Mon, Jul 18, 2016 at 10:50 AM, Matt Riedemann<br>
<<a href="mailto:mriedem@linux.vnet.ibm.com" target="_blank">mriedem@linux.vnet.ibm.com</a>>  wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
On 7/18/2016 9:42 AM, Matt Riedemann wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
On 7/18/2016 9:09 AM, Markus Zoeller wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Since yesterday, Nova uses "oslo.context" 2.6.0 [1] but the needed<br>
change [2] is not yet in place, which broke "gate-nova-python27-db"[3].<br>
Logstash counts 70 hits/h [4]. Most folks will be at the midcycle in<br>
Portland and won't be available for the next 2h or so.<br>
If you can have a look at it and merge it, that would be great.<br>
<br>
References:<br>
[1]<br>
<br>
<a href="https://github.com/openstack/requirements/commit/238389c4ee1bd3cc9be4931dd2639aea2dae70f1" rel="noreferrer" target="_blank">https://github.com/openstack/requirements/commit/238389c4ee1bd3cc9be4931dd2639aea2dae70f1</a><br>
<br>
[2] <a href="https://review.openstack.org/#/c/342604/1" rel="noreferrer" target="_blank">https://review.openstack.org/#/c/342604/1</a><br>
[3] <a href="https://bugs.launchpad.net/nova/+bug/1603979" rel="noreferrer" target="_blank">https://bugs.launchpad.net/nova/+bug/1603979</a><br>
[4] logstash: <a href="http://goo.gl/79yFb9" rel="noreferrer" target="_blank">http://goo.gl/79yFb9</a><br>
<br>
</blockquote>
This is an API change for oslo.context, why wasn't it released as 3.0.0?<br>
<br>
Seems we should revert the upper-constraints bump and blacklist 2.6.0,<br>
then get that released as 3.0.0.<br>
<br>
</blockquote>
Here is the blacklist:<br>
<br>
<a href="https://review.openstack.org/#/c/343683/" rel="noreferrer" target="_blank">https://review.openstack.org/#/c/343683/</a><br>
<br>
<br>
--<br>
<br>
Thanks,<br>
<br>
Matt Riedemann<br>
<br>
<br>
__________________________________________________________________________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" rel="noreferrer" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" rel="noreferrer" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
</blockquote>
__________________________________________________________________________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" rel="noreferrer" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" rel="noreferrer" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
<br>
</blockquote>
<br>
<br>
__________________________________________________________________________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" rel="noreferrer" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" rel="noreferrer" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
</blockquote>
<br>
__________________________________________________________________________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" rel="noreferrer" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" rel="noreferrer" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
</div></div></blockquote></div><br></div></div>