Review the inheritance architechture

[diraol]

We have found a big problem regarding the inheritance between versions.

The first plan was to import the classes from a previous version if they haven't changed on the spec between both versions. So, for example, on v0x01 we have: v0x01/symmetric/echo_request.py

from pyof.v0x01.common.header import Header, Type
from pyof.v0x01.foundation.base import GenericMessage

class EchoRequest(GenericMessage):
    header = Header(message_type=Type.OFPT_ECHO_REQUEST, length=8)

As the message EchoRequest has the same structure on the version v0x02, we expected to reuse the class from v0x01, so we would have: v0x02/symmetric/echo_request.py

from pyof.v0x01.symmetric.echo_request import EchoRequest

One of the problems is related to the value passed to the header. So, for example:

>>> from pyof.v0x01.symmetric.echo_request import EchoRequest as ER1
>>> from pyof.v0x02.symmetric.echo_request import EchoRequest as ER2
>>> # Now let's create one message for each class:
>>> msg1 = ER1()
>>> msg2 = ER2()
>>> # Now we want to see the openflow version of each message:
>>> msg1.header.version
UBInt8(1)
>>> msg2.header.version
UBInt8(1)
>>> # As we can see, the version on the message from v0x02 is '1',
>>> # because it is inherited from v0x01.

The same behavior would happen for any attribute that have changed between versions and also for any message that the Type has changed (because a new Type was inserted on the enum before its options).

Another problem is that if we try to create subclasses between versions, the subclass will loose the __ordered__ dict from its superclass, and we need that attribute.

All in all, we need to review how the inheritance for evolutive reuse can be done.

For now, we have decided to fully implement the versions without inheritance between versions.

[diraol]

Please @cemsbr, @abaruchi, @beraldoleal and @raphaelmcobe help improve the above content so it is absolutely clear to everyone. If we let something pass here, on the future, when we came back to this issue, we won't remember everything and understand all the problems that need to be handled.

[cemsbr]

Thanks for the clarification, I haven't noticed the __ordered__ problem yet. In some issue I think I wrote that if an Enum or Bitmask is changed, we implement it all again and every class that uses them. If an unchanged class uses an updated Enum and we import the class from v0x01, it will use the v0x01 Enum (because it imports its own Enum). Thus, whenever a class, Enum, Bitmask, etc changes, we should re-implement it and whatever files that depends on it.

However, if we find a module that has no changes, I would consider creating the module file in v0x02 that only imports the v0x01 version so the user can import anything from pyof.v0x02 (I haven't found such cases yet and they are expected to happen after all v0x02 changes are implemented). But I'll analyze it carefully before trying not to duplicate code.

[diraol]

I've been thinking about this inheritance issue, and I still cannot see an easy and clean way of addressing it.

It may be addressed by the MetaStruct metaclass, with some logic evaluating the Parent class (cls.__bases__), looking for the __ordered__ attribute.

We have some cases to think about:

1. Add a new attribute at the end of the new class;
2. Remove an existing attribute from the parent class;
3. Change an existing attribute from the parent class (but keeping it on the same position);
4. Move an attribute from position A to position B;
5. Add a new attribute before/after another existing attribute;
6. Rename an existing attribute;

One possible solution, to keep our GenericStruct/GenericMessage classes as they are, is to just (deep)copy the __ordered__ attribute from the parent class (if it have that attribute) and modify the Child class attributes using decorators (@addAttribute(attr_name, attr_class), @addAttributeAt(attr_name, attr_class, index), @removeAttribute(name), @moveAttributeAfter(name, attr_ref_name), @renameAttribute(name, new_name), etc).

Another approach would be to forget about our "orderedDict" and create a new attribute that will be a list with the name of the class attributes in the expected order.

These are just some ideas and insights I have had while trying to solve this problem. More ideas are welcome and I think that we need to have a deeper discussion on this matter, since it is a critical issue related to the evolution of the library.

Cheers!

[cemsbr]

On Wed, Dec 21, 2016 at 3:15 PM, Diego Rabatone Oliveira < [email protected]> wrote:

I've been thinking about this inheritance issue, and I still cannot see an easy and clean way of addressing it.

It may be addressed by the MetaStruct metaclass, with some logic evaluating the Parent class (cls.bases), looking for the ordered attribute.

We have some cases to think about:

1. Add a new attribute at the end of the new class;
2. Remove an existing attribute from the parent class;
3. Change an existing attribute from the parent class (but keeping it on the same position);
4. Move an attribute from position A to position B;
5. Add a new attribute before/after another existing attribute;
6. Rename an existing attribute;

One possible solution, to keep our GenericStruct/GenericMessage classes as they are, is to just (deep)copy the ordered attribute from the parent class (if it have that attribute) and modify the Child class attributes using decorators (@addAttribute(attr_name, attr_class), @addAttributeAt(attr_name, attr_class, index), @removeAttribute(name), @moveAttributeAfter(name, attr_ref_name), @renameAttribute(name, new_name), etc).

Another approach would be to forget about our "orderedDict" and create a new attribute that will be a list with the name of the class attributes in the expected order.

I prefer this KISS way. It will be easier to manipulate the attribute order and for newcomers to understand the code.

These are just some ideas and insights I have had while trying to solve this problem. More ideas are welcome and I think that we need to have a deeper discussion on this matter, since it is a critical issue related to the evolution of the library.

Cheers!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/kytos/python-openflow/issues/157#issuecomment-268579921, or mute the thread https://github.com/notifications/unsubscribe-auth/AADBSbAAE9MrK_K2W-Xb_e6dksXGl42bks5rKV7MgaJpZM4J2A2F .

[cemsbr]

Please, check https://github.com/kytos/python-openflow/wiki/Version-Inheritance. I've just added a solution to this problem among other ones.