On Wed, Apr 18, 2018 at 10:31 AM, Robert Varga <[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".
>
​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]))';
      }
    }​

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)

public class Ipv4AddressNoZone extends Ipv4Address

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.

Given that Tom/Jei reviewed this patch, just want to get their thoughts as
well ?


> Sent from my BlackBerry - the most secure mobile device - via the Orange
> Network
> *From:* [email protected]
> *Sent:* April 18, 2018 7:12 PM
> *To:* [email protected]
> *Cc:* [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]
> *Subject:* Re: Build breakage in openflowplugin due IP address NoZone
> changes
>
> Can you please explain it a bit, i am not sure what do you mean by
> "inferred from anything machine-readable"?
>
> On Wed, Apr 18, 2018 at 10:07 AM, Robert Varga <[email protected]> wrote:
>
>> Unfortunately that cannot be inferred from anything machine-readable, so
>> we have to make do with what we have.
>>
>> Sent from my BlackBerry - the most secure mobile device - via the Orange
>> Network
>> *From:* [email protected]
>> *Sent:* April 18, 2018 6:57 PM
>> *To:* [email protected]
>> *Cc:* [email protected]; [email protected];
>> [email protected]; [email protected];
>> [email protected]
>> *Subject:* Re: Build breakage in openflowplugin due IP address NoZone
>> changes
>>
>>
>>
>> On Wed, Apr 18, 2018 at 9:49 AM, Robert Varga <[email protected]> wrote:
>>
>>> Hello Anil,
>>>
>>> No, support is not being removed, but Ipv4Address.equals() is defined
>>> in a way which returns false when compared with Ipv4AddressNoZone of the
>>> same value IIRC, hence things would break.
>>>
>> ​This is not correct behavior in my opinion, because ietf clearly says
>> that user "may" add zone, so the equals should not fail if two ipv4-address
>> (without zone) is compared.​
>>
>>
>>>
>>> Yes, applications can support zones and conversion from NoZone is cheap,
>>> so it is a matter of preference.
>>>
>>> Sent from my BlackBerry - the most secure mobile device - via the Orange
>>> Network
>>> *From:* [email protected]
>>> *Sent:* April 18, 2018 6:13 PM
>>> *To:* [email protected]
>>> *Cc:* [email protected]; [email protected];
>>> [email protected]; [email protected];
>>> [email protected]
>>> *Subject:* Re: Build breakage in openflowplugin due IP address NoZone
>>> changes
>>>
>>> Hi Robert,
>>>
>>> Please see inline..
>>>
>>> On Wed, Apr 18, 2018 at 1:33 AM, Robert Varga <[email protected]> wrote:
>>>
>>>> On 18/04/18 08:09, D Arunprakash wrote:
>>>> > Hello,
>>>> >
>>>> > The following review in mdsal might have impacted openflowplugin
>>>> > functionality.
>>>> >
>>>> > https://git.opendaylight.org/gerrit/#/c/70769/
>>>>
>>>> Sorry about that.
>>>>
>>>> > https://jenkins.opendaylight.org/releng/job/openflowplugin-m
>>>> aven-verify-fluorine-mvn33-openjdk8/259/console
>>>> >
>>>> >
>>>> >
>>>> > 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}>
>>>> >
>>>> > is it new expectation on openflowplugin to change from Ipv4Address to
>>>> > Ipv4AddressNoZone ?
>>>>
>>>> There are two aspects to this.
>>>>
>>>> As an interim, the codecs from wire need to be updated to convert
>>>> Ipv4AddressNoZone to Ipv4Address, so that equality works as expected and
>>>> the breakage is recovered -- https://git.opendaylight.org/gerrit/71072
>>>> does that.
>>>>
>>> ​This seems like regression. ietf-type (2013-07-15) supports both the
>>> version and this revision seems to be backward compatible. So are you
>>> removing the support for ipv4-address type? ​
>>>
>>>
>>>>
>>>> Going forward, though, I believe the openflow models need to be updated
>>>> to require ipv4-address-no-zone rather than ipv4-address (and same goes
>>>> for ip-address and ipv6-address). This really is the correct thing to do
>>>> -- ipv4-address is not really the IPv4 address used in OpenFlow
>>>> protocol.
>>>>
>>> ​Yeah for flows /groups true, but that does not mean application can't
>>> use ipv4_address with zone.​
>>>
>>>
>>>>
>>>> Regards,
>>>> Robert
>>>>
>>>>
>>>
>>>
>>> --
>>> Thanks
>>> Anil
>>>
>>
>>
>>
>> --
>> Thanks
>> Anil
>>
>
>
>
> --
> Thanks
> Anil
>



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

Reply via email to