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- > verify-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
