[openstack-dev] [Murano] Improvements to MuranoPL contracts
Stan Lagun
slagun at mirantis.com
Tue May 27 12:51:33 UTC 2014
You're correct for not-nullable contract types. I agree that it is less
obvious. But on the other side it simplifies contract understanding because
otherwise developer needs to know not only how null is treated but also how
NoValue is treated. This also make UI development easier as with null equal
to NoValue you can have templated object model that you substitute values
from web form into. With nulls the structure always remain the same only
values change while with NoValue you have to delete key from template to
get default.
What is better? I don't know. Lets wait for other opinions
Sincerely yours,
Stan Lagun
Principal Software Engineer @ Mirantis
<slagun at mirantis.com>
On Tue, May 27, 2014 at 4:29 PM, Alexander Tivelkov
<ativelkov at mirantis.com>wrote:
> Got it.
> What about this "it is easier to set attribute value to None rather then
> deleting attribute from object if default value is desired, especially for
> generic clients"?
> Do I understand correctly that setting some property to None should leads
> to automatic assignment of the default value?
>
> As for me, this looks not obvious, and I would prefer not to have such
> implicit assignments
>
> --
> Regards,
> Alexander Tivelkov
>
>
> On Tue, May 27, 2014 at 4:05 PM, Stan Lagun <slagun at mirantis.com> wrote:
>
>> Hi Alex,
>>
>> 1. This is directly related to the bug mentioned above.
>>
>> Taking an example from launchpad
>>
>> networks:
>> Contract:
>> environment: $.bool().notNull()
>> flat: $.bool().notNull()
>> custom: [$.class(Network).notNull()]
>>
>> Default:
>> environment: true
>> flat: false
>>
>>
>> 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
>>
>> networks:
>> Contract:
>> environment: $.bool()
>> flat: $.bool()
>> custom: [$.class(Network)]
>>
>> Default:
>> environment: true
>>
>> 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.
>> 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
>>
>>
>> Sincerely yours,
>> Stan Lagun
>> Principal Software Engineer @ Mirantis
>>
>> <slagun at mirantis.com>
>>
>>
>> On Tue, May 27, 2014 at 1:37 PM, Alexander Tivelkov <
>> ativelkov at mirantis.com> wrote:
>>
>>> Hi Stan,
>>>
>>> I don't understand well your third problem/solution statement. Could you
>>> please give an example of the proposed behaviour vs the current one?
>>>
>>> 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.
>>>
>>> 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.
>>>
>>>
>>>
>>> --
>>> Regards,
>>> Alexander Tivelkov
>>>
>>>
>>> On Mon, May 26, 2014 at 6:40 PM, Stan Lagun <slagun at mirantis.com> wrote:
>>>
>>>> Hello everyone!
>>>>
>>>> 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:
>>>>
>>>> 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
>>>>
>>>> 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.
>>>>
>>>> ---
>>>>
>>>> Problem: Single default value is not enough for properties that are not
>>>> of scalar type.
>>>> For example if we have contract like
>>>> - name: $.string()
>>>> disabled: $.bool()
>>>>
>>>> (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.
>>>>
>>>> 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.
>>>>
>>>> ---
>>>>
>>>> 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.
>>>>
>>>> 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)
>>>>
>>>> ---
>>>>
>>>> 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.
>>>>
>>>> Solution: to make all primitive types be not-nullable with obvious
>>>> defaults (0 for int, false for bool).
>>>> 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.
>>>>
>>>> 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.
>>>>
>>>> ---
>>>>
>>>> While some of proposed changes can possible break existing contracts in
>>>> practice they just legalize current state of affairs so nothing would break.
>>>>
>>>> What do you think?
>>>>
>>>> [1]: https://bugs.launchpad.net/murano/+bug/1313694
>>>>
>>>> Sincerely yours,
>>>> Stan Lagun
>>>> Principal Software Engineer @ Mirantis
>>>>
>>>> <slagun at mirantis.com>
>>>>
>>>> _______________________________________________
>>>> OpenStack-dev mailing list
>>>> OpenStack-dev at lists.openstack.org
>>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>>>
>>>>
>>>
>>> _______________________________________________
>>> OpenStack-dev mailing list
>>> OpenStack-dev at lists.openstack.org
>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>>
>>>
>>
>> _______________________________________________
>> OpenStack-dev mailing list
>> OpenStack-dev at lists.openstack.org
>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>
>>
>
> _______________________________________________
> OpenStack-dev mailing list
> OpenStack-dev at lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20140527/b0d8e216/attachment.html>
More information about the OpenStack-dev
mailing list