<div dir="ltr"><div><div><div><div>Hi Alex,<br><br></div>1. This is directly related to the bug mentioned above.<br><br></div>Taking an example from launchpad<br><br>networks:<br>
Contract:<br>
environment: $.bool().notNull()<br>
flat: $.bool().notNull()<br>
custom: [$.class(Network).notNull()]<br><p>
Default:<br>
environment: true<br>
flat: false</p><p><br></p><p>currently if no value was passed for 'networks' property (and no value means no such key, null value doesn't count) then default value would be used ({"environment": true, "flat": false}) and contract would be applied to that value. If you pass "networks": {"flat": "true"} you will get exception as this value doesn't matched contract (for 'environment' key). With new approach exactly the same Default is interpreted as a set of independent default - default for 'environment' attribute and default for 'flat' attribute. So {"flat": "true"} turns into {"environment": "true", "flat": true}. With proposed changes the example above can be rewritten as<br>
</p><p>networks:<br>
Contract:<br>
environment: $.bool()<br>
flat: $.bool()<br>
custom: [$.class(Network)]<br></p><p>
Default:<br>
environment: true</p><br></div>2. Yes, I do agree it breaks compatibility. I believe we will brake it anyway because MuranoPL is still far from mature and it is very likely that some new future would require incompatible change. Thats was one of the purpose of app-incubator repository - if we make breaking changes we can walk through all applications and fix them. I hope that all such changes will be made in Juno time frame. <br>
</div>As for this particular change it is 1% chance that $.int() contract was intentionally written to accept null and 99% that it was mistake. So making $.int() not-nullable we do much more good than cause harm<br><div>
<div>
<div><br><div><div><div><div class="gmail_extra"><br clear="all"><div><div dir="ltr"><span style="border-collapse:separate;color:rgb(0,0,0);font-family:'Times New Roman';font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;font-size:medium"><span style="font-family:arial;font-size:small">Sincerely yours,<br>
Stan Lagun<br>Principal Software Engineer @ Mirantis</span></span><br><span style="border-collapse:separate;color:rgb(0,0,0);font-family:'Times New Roman';font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;font-size:medium"><span style="font-family:arial;font-size:small"><br>
<a href="mailto:slagun@mirantis.com" target="_blank"></a></span></span></div></div>
<br><br><div class="gmail_quote">On Tue, May 27, 2014 at 1:37 PM, Alexander Tivelkov <span dir="ltr"><<a href="mailto:ativelkov@mirantis.com" target="_blank">ativelkov@mirantis.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">
<div dir="ltr"><div style="font-size:small">Hi Stan,</div><div style="font-size:small"><br></div><div style="font-size:small">I don't understand well your third problem/solution statement. Could you please give an example of the proposed behaviour vs the current one?</div>
<div style="font-size:small"><br></div><div style="font-size:small">The last solution - to have not-nullable primitive types by default - looks good to me, but it breaks the compatibility with the existing contracts. If some package developers have already got "$.int()" in their classes, we cannot be sure if this is a mistake or an intention to make the property nullable. In the latter case their code will be broke, as the null values will not be accepted anymore. </div>
<div style="font-size:small"><br></div><div style="font-size:small">I understand that currently the adoption of MuranoPL is quite low and most of its users are murano-contributors and so are probably reading this mailing list, so this breaking change should not surprise them, but I'd rather be careful with such things anyway. I suggest to discuss this today on a weekly meeting in IRC. If everybody are fine with such changes, then let's do them, as they are reasonable. </div>
<div style="font-size:small"><br></div><div style="font-size:small"><br></div></div><div class="gmail_extra"><br clear="all"><div><div dir="ltr"><font>--<br></font><div dir="ltr">
<font>Regards,<br>Alexander Tivelkov</font></div></div></div>
<br><br><div class="gmail_quote"><div><div class="h5">On Mon, May 26, 2014 at 6:40 PM, Stan Lagun <span dir="ltr"><<a href="mailto:slagun@mirantis.com" target="_blank">slagun@mirantis.com</a>></span> wrote:<br></div>
</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div class="h5">
<div dir="ltr"><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div>Hello everyone!<br><br></div>Recently I've been looking for a best way to fix incorrect handling of defaults in MuranoPL's property contracts [1]. I analyzed how contracts used in applications that are currently in app-incubator, typical mistakes and usage patterns. I've found number of places where contract system can be significantly improved. Here is my list of proposed improvements I'd like to deliberate with you:<br>
<br></div>Problem: when contract violation happens it is very hard to tell without debugger what went wrong. Especially this is a problem for complex nested structures. Currently it even impossible to tell which property caused exception during class load<br>
<br></div>Solution: during contract traversal keep track of path to a part of contract being processed (for example 'foo/0/bar'). Include that path in every thrown exception alongside with human-readable description describing what value was processed and what contract was violated. Prepend property name to exception text so that is would be clear what property cannot be initialized. The same for method arguments.<br>
<br>---<br><br></div>Problem: Single default value is not enough for properties that are not of scalar type.<br></div>For example if we have contract like<br></div></div>- name: $.string()<br></div> disabled: $.bool()<br>
<br></div>
(array of structures) it is reasonable to have default value for "disabled" attribute of each list entry whereas there is no meaningful default for "name" attribute. Current approach allows to provide initial filling of entire array which is obviously not what developer expects.<br>
<br></div>Solution: to have Default reflect contract structure so that different parts of Default can serve as an independent defaults for corresponding parts of the contract. Default need to be traversed in parallel with contract spec and provided value. Default value need not provide value for every single attribute mentioned in contract spec and thus can be simpler than contract itself.<br>
<br>---<br><br></div>Problem: Default value is only used when no property value provided at all. Null (None) value is a valid value even if it not valid for particular contract. Thus is null is passed Default is not used.<br>
<br></div>Solution: to treat every missing value as null. Missing Default is also considered to be null. This greatly simplifies understanding of contracts and client development (it is easier to set attribute value to None rather then deleting attribute from object if default value is desired, especially for generic clients)<br>
<br>---<br><br></div>Problem: most of the time developer writes $.int() contract he doesn't realize that null is also valid value for such contract and the correct form is $.int().notNull(). This is even more obvious in case of bool(). Developers are lazy and usually forget to wright long verbose contracts.<br>
</div><br>Solution: to make all primitive types be not-nullable with obvious defaults (0 for int, false for bool).<br></div>This is enough for 95% of use cases. For the rest 5% where null value does valid to have separate contract methods optionalInt(), optionalBool() etc.<br>
<br></div>As for strings I think that the same approach should be used - $.string() is a non-nullable sequence of chars with empty string as a default. And there should also be optionalString() that es equal to current $.string(). But in most cases when developer writes $.string() what he really wants is $.string().notNull().trim().notEmpty(). So while it is reasonable to have all this helper functions (notEmpty(), notBlank(), trim() etc.) is would be great to have one contract function that does exactly that.<br>
<br></div>---<br><br>While some of proposed changes can possible break existing contracts in practice they just legalize current state of affairs so nothing would break.<br><br></div>What do you think?<br>
<div><div><div><div><div><div><div><div><div><div><div><div><br><div><div><div><div><div><div><div><div><div><div><div><div><div><div>[1]: <a href="https://bugs.launchpad.net/murano/+bug/1313694" target="_blank">https://bugs.launchpad.net/murano/+bug/1313694</a><br>
<br clear="all"><div><div dir="ltr">
<span style="border-collapse:separate;color:rgb(0,0,0);font-family:'Times New Roman';font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;font-size:medium"><span style="font-family:arial;font-size:small">Sincerely yours,<br>
Stan Lagun<br>Principal Software Engineer @ Mirantis</span></span><br><span style="border-collapse:separate;color:rgb(0,0,0);font-family:'Times New Roman';font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;font-size:medium"><span style="font-family:arial;font-size:small"><br>
<a href="mailto:slagun@mirantis.com" target="_blank"></a></span></span></div></div>
</div></div></div></div></div></div></div></div></div></div></div></div></div></div></div></div></div></div></div></div></div></div></div></div></div></div></div>
<br></div></div>_______________________________________________<br>
OpenStack-dev mailing list<br>
<a href="mailto:OpenStack-dev@lists.openstack.org" target="_blank">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>
<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></div></div></div></div></div></div>