Robert, please respond to this thread, so others also get some clarity on
it, so you don't have to answer all the projects ;)

On Wed, Apr 18, 2018 at 2:02 PM, Anil Vishnoi <[email protected]> wrote:

>
>
> On Wed, Apr 18, 2018 at 1:13 PM, Robert Varga <[email protected]> wrote:
>
>> On 18/04/18 21:17, Anil Vishnoi wrote:
>> >
>> >
>> > On Wed, Apr 18, 2018 at 10:31 AM, Robert Varga <[email protected]
>> > <mailto:[email protected]>> wrote:
>> >
>> >     Object.equals() is required to be reflexive. Your statement "this is
>> >     not correct behavior" is based on your human understanding of the
>> >     model (most notably reading English text in description), not
>> >     something automation can infer. Therefore codegen cannot generate an
>> >     .equals() method, which would be both conforming to the API contract
>> >     and supporting "correct behavior".
>> >
>>
>> Hello Anil,
>>
>> > ​Sorry robert, but i am not able to make sense out of what you are
>> > saying. English language can be interpreted in many ways, but hopefully
>> > you will agree that patterns has clear interpretation. Both ipv4-address
>> > and ipv4-address-no-zone has clear pattern defined.
>> >
>> > typedef ipv4-address-no-zone {
>> >       type ipv4-address {
>>         ^^^^^^^^^^^^^^^^^
>>
>> >         pattern '[0-9\.]*';
>> >       }
>> >     }
>> >  ​
>> >
>> > ​ typedef ipv4-prefix {
>> >       type string {
>> >         pattern
>> >           '(([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])\.){3}'
>> >             + '([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])'
>> >             + '/(([0-9])|([1-2][0-9])|(3[0-2]))';
>> >       }
>> >     }​
>>
>> Wrong type, you meant to quote this:
>>
>> >   typedef ipv4-address {
>> >     type string {
>> >       pattern
>> >         '(([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])\.){3}'
>> >       +  '([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])'
>> >       + '(%[\p{N}\p{L}]+)?';
>> >     }
>>
>> but aside from that, yes those patterns are clear. What is clear to
>> humans, but not machines, is that:
>>
>> 1) an ipv4-address string really has two parts, a dotted-quad and an
>> optional zone prefixed with %
>>
>> 2) ipv4-address-no-zone is an ipv4-address, which does not have a zone
>>
>> Arriving at this distinction by analyzing the patterns involved is the
>> challenge here -- as those are the only bits which a
>> machine-understandable. I am pretty sure an algorithm to solve that
>> problem, if it is even possible, would run in non-linear time and would
>> probably be a major breakthrough in information technology.
>>
> ​Given that these pattern are already mapped to certain derive type, i
> assuming ​you don't have to do any breakthrough here to differentiate
> ipv4-address and ipv4-address-no-address, but i think the problem here is
> the way it's derived in the yang model itself.
>
>>
>> > And apart from that the failure is happening because equals is expecting
>> > different class then ipv4address (which is changed in the above
>> gerrit).
>> > org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.iet
>> f.inet.types.rev130715.Ipv4Address<Ipv4Address{_value=0.1.2.3}>
>> > but was:
>> > org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.iet
>> f.inet.types.rev130715.Ipv4AddressNoZone<Ipv4Address{_value=0.1.2.3}>
>> >
>> >
>> > I believe this is most probably happening because the way mdsal is
>> > generating the code now for no-zone in your latest patch.
>> >
>> > Currently generated code for ipv4-address-no-zone is extending
>> > ipv4-addresss (which is weird to me , subset extending superset)
>>
>> You have to understand how YANG type system works, which is detailed in
>> RFC7950 sections 4.2.4, 4.2.5, 7.3 and 7.4.
>>
>> TL;DR; of that is that all YANG types are derived from a set of fixed
>> built-in types. Derivation works by placing additional restrictions on
>> the data, not extending the data set -- which is where the weirdness you
>> perceive comes from.
>>
>> In in any case
>>
>> > public class Ipv4AddressNoZone extends Ipv4Address
>>
>> is an accurate mapping of the semantic relationship between the two types:
>>
>> -
>> ​​
>> any valid Ipv4AddressNoZone is a valid Ipv4Address, but not vice-versa.
>> - creating an Ipv4Address from an Ipv4AddressNoZone is dirt cheap
>> - creating an Ipv4AddressNoZone from an Ipv4Address requires validation
>>
>> It is therefore clear that IetfInetUtil should be handing out
>> Ipv4AddressNoZone objects -- as that is the semantically correct type
>> capture of what is inside an Inet4Address.getAddress() array.
>>
>> > Given that both ipv4-address-no-zone and ipv4-address are different
>> > construct by definition, if mdsal generates these construct separately
>> > (without extending) that should keep the backward compatibility without
>> > breaking any downstream code.
>>
>> They are not completely different constructs, they are related through
>> derivation:
>> - string is the base type of ipv4-address
>> - ipv4-address is a type derived from string
>> - ipv4-address-no-zone is a type derived from ipv4-address
>>
> ​Okay understood, it also means application can use these two derive types
> individually and should be comparable ? If that is the case, why the equal
> is failing in below line, given that match.getNwSrc() is returning
> IPv4Address?
>
>         Assert.assertEquals("Wrong nw-src", new
> Ipv4Address("16.17.18.19"), match.getNwSrc());
> ​
>
> ​I believe it's because internally for all the ip-address type it's
> generating IPv4AddressNoZone object? Also why i have to change my yang
> model to not use ipv4-address?
>
> https://git.opendaylight.org/gerrit/#/c/71083/3/extension/
> openflowjava-extension-nicira/src/main/yang/nicira-action.yang
>
> This is kind of causing an issue here in the overall understanding of
> using ipv4-address and ipv4-address-no-zone. There are two things that you
> mentioned
> (1) This is not conforming to the API contract but it's "correct" behavior.
> (2)
> ​
> any valid Ipv4AddressNoZone is a valid Ipv4Address, but not vice-versa.
>
> (1) is going to introduce a confusion for the user here because of the way
> mdsal handles it and reading (2) basically says you don't really need
> ipv4-address-no-zone, you can use ipv4-address for ipv4-address-no-zone as
> well.
>
> Based on these data points, i believe we should better not support
> ipv4-address-no-zone and suggest user to use ipv4-address, rather then
> introducing this feature and confusing users on how they should write the
> code around it.
>
>
>> Regards,
>> Robert
>>
>>
>
>
> --
> Thanks
> Anil
>



-- 
Thanks
Anil
_______________________________________________
openflowplugin-dev mailing list
[email protected]
https://lists.opendaylight.org/mailman/listinfo/openflowplugin-dev

Reply via email to