Thanks Anil. Best, Vina
From: <[email protected]<mailto:[email protected]>> on behalf of Anil Vishnoi <[email protected]<mailto:[email protected]>> Date: Wednesday, April 18, 2018 at 2:18 PM To: Robert Varga <[email protected]<mailto:[email protected]>> Cc: "[email protected]<mailto:[email protected]>" <[email protected]<mailto:[email protected]>>, "[email protected]<mailto:[email protected]>" <[email protected]<mailto:[email protected]>>, Release <[email protected]<mailto:[email protected]>>, "[email protected]<mailto:[email protected]>" <[email protected]<mailto:[email protected]>>, D Arunprakash <[email protected]<mailto:[email protected]>> Subject: Re: [release] Build breakage in openflowplugin due IP address NoZone changes 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]<mailto:[email protected]>> wrote: On Wed, Apr 18, 2018 at 1:13 PM, Robert Varga <[email protected]<mailto:[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]> > <mailto:[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.ietf.inet.types.rev130715.Ipv4Address<Ipv4Address{_value=0.1.2.3}> > but was: > org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.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
