Hi Robert, Could you please take a look at tell what I need to fix this breakage? I am still not sure why is such a basic code breaking like this.
Regards, Vishal. On Thu, Apr 19, 2018 at 5:38 PM, Vishal Thapar <[email protected]> wrote: > And it still failed with the 'fix' > > https://jenkins.opendaylight.org/releng/job/ovsdb-maven- > verify-fluorine-mvn33-openjdk8/90/console > > Regards, > Vishal. > > On Thu, Apr 19, 2018 at 4:36 PM, Vishal Thapar <[email protected]> wrote: > >> I'm seeing similar issue in OVSDB too [1] and and have raised following >> fix for it [2]. This looks like a API change and should've been >> communicated better, maybe weather page and heads up to all consumers? >> >> Regards, >> Vishal. >> >> [1] https://jenkins.opendaylight.org/releng/job/ovsdb-maven-veri >> fy-fluorine-mvn33-openjdk8/89/console >> [2] https://git.opendaylight.org/gerrit/#/c/71105/ >> >> >> >> On Thu, Apr 19, 2018 at 2:47 PM, Robert Varga <[email protected]> wrote: >> >>> On 18/04/18 23:18, Anil Vishnoi wrote: >>> > 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.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? >>> >>> it is *declared* to return Ipv4Address, but returns a subclass. >>> >>> > 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/o >>> penflowjava-extension-nicira/src/main/yang/nicira-action.yang >>> > <https://git.opendaylight.org/gerrit/#/c/71083/3/extension/ >>> openflowjava-extension-nicira/src/main/yang/nicira-action.yang> >>> >>> You do not have to, but... >>> >>> > 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. >>> >>> ... it is matter of what is the correct model. Given that OpenFlow has >>> no notion of a zone, I believe ipv4-address-no-zone is the correct type >>> representing OFPXMT_OFB_IPV4_*. To illustrate, can you explain what >>> happens when a user specifies two matches, which differ only in zone, >>> for example "1.2.3.4%a" and "1.2.3.4%b"? >>> >>> Regards, >>> Robert >>> >>> >>> _______________________________________________ >>> release mailing list >>> [email protected] >>> https://lists.opendaylight.org/mailman/listinfo/release >>> >>> >> >
_______________________________________________ openflowplugin-dev mailing list [email protected] https://lists.opendaylight.org/mailman/listinfo/openflowplugin-dev
