[ovs-dev,v1,03/18] python: add list parser

Some openflow or dpif flows encode their arguments in lists, eg:
"some_action(arg1,arg2,arg3)". In order to decode this in a way that can
be then stored and queried, add ListParser and ListDecoders classes
that parse lists into KeyValue instances.

The ListParser / ListDecoders mechanism is quite similar to KVParser and
KVDecoders. Since the "key" of the different KeyValue objects is now
ommited, it has to be provided by ListDecoders.

For example, take the openflow action "resubmit" that can be written as:

resubmit([port],[table][,ct])

Can be decoded by creating a ListDecoders instance such as:

ListDecoders([
                ("port", decode_default),
                ("table", decode_int),
                ("ct", decode_flag),
            ])

Naturally, the order of the decoders must be kept.

Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
---
 python/automake.mk       |   3 +-
 python/ovs/flows/list.py | 123 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 125 insertions(+), 1 deletion(-)
 create mode 100644 python/ovs/flows/list.py

See some minor comment below.

//Eelco


On 22 Nov 2021, at 12:22, Adrian Moreno wrote:

> Some openflow or dpif flows encode their arguments in lists, eg:
> "some_action(arg1,arg2,arg3)". In order to decode this in a way that can
> be then stored and queried, add ListParser and ListDecoders classes
> that parse lists into KeyValue instances.
>
> The ListParser / ListDecoders mechanism is quite similar to KVParser and
> KVDecoders. Since the "key" of the different KeyValue objects is now
> ommited, it has to be provided by ListDecoders.
>
> For example, take the openflow action "resubmit" that can be written as:
>
> resubmit([port],[table][,ct])
>
> Can be decoded by creating a ListDecoders instance such as:
>
> ListDecoders([
>                 ("port", decode_default),
>                 ("table", decode_int),
>                 ("ct", decode_flag),
>             ])
>
> Naturally, the order of the decoders must be kept.
>
> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
> ---
>  python/automake.mk       |   3 +-
>  python/ovs/flows/list.py | 123 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 125 insertions(+), 1 deletion(-)
>  create mode 100644 python/ovs/flows/list.py
>
> diff --git a/python/automake.mk b/python/automake.mk
> index 13aa2b4c3..a3265292d 100644
> --- a/python/automake.mk
> +++ b/python/automake.mk
> @@ -44,7 +44,8 @@ ovs_pyfiles = \
>  	python/ovs/winutils.py \
>  	python/ovs/flows/__init__.py \
>  	python/ovs/flows/decoders.py \
> -	python/ovs/flows/kv.py
> +	python/ovs/flows/kv.py \
> +	python/ovs/flows/list.py
>
>  # These python files are used at build time but not runtime,
>  # so they are not installed.
> diff --git a/python/ovs/flows/list.py b/python/ovs/flows/list.py
> new file mode 100644
> index 000000000..d7ad315a6
> --- /dev/null
> +++ b/python/ovs/flows/list.py
> @@ -0,0 +1,123 @@
> +import re
> +import functools
> +
> +from ovs.flows.kv import KeyValue, KeyMetadata, ParseError
> +from ovs.flows.decoders import decode_default
> +
> +
> +class ListDecoders:
> +    """ListDecoders is used by ListParser to decode the elements in the list

Please add dots here, and to the other comments.

> +    A decoder is a function that accepts a value and returns its decoded
> +    object
> +    The list_decoder to be used is determined by index in the list provided to
> +    ListDecoders is important.

This last sentence does not make sense.

> +
> +    Args:
> +        decoders (list of tuples): Optional,  A list of tuples.
> +            The first element in the tuple is the keyword associated with the
> +            value. The second element in the tuple is the decoder function.
> +    """
> +
> +    def __init__(self, decoders=None):
> +        self._decoders = decoders or list()
> +
> +    def decode(self, index, value_str):
> +        """Decode the index'th element of the list
> +
> +        Args:
> +            index (int): the position in the list of the element ot decode

Guess ot, should be to.

> +            value_str (str): the value string to decode
> +        """
> +        if index < 0 or index >= len(self._decoders):
> +            return self._default_decoder(index, value_str)
> +
> +        try:
> +            key = self._decoders[index][0]
> +            value = self._decoders[index][1](value_str)
> +            return key, value
> +        except Exception as e:
> +            raise ParseError(
> +                "Failed to decode value_str %s: %s" % (value_str, str(e))

Personally, I try to move away from % formatting and use .format() but not sure what the general rule for OVS is.
I know there now also is f””, but I’m not using that yet.

> +            )
> +
> +    @staticmethod
> +    def _default_decoder(index, value):
> +        key = "elem_{}".format(index)
> +        return key, decode_default(value)
> +
> +
> +class ListParser:
> +    """ListParser parses a list of values and stores them as key-value pairs
> +
> +    It uses a ListDecoders instance to decode each element in the list.
> +
> +    Args:
> +        decoders (ListDecoders): Optional, the decoders to use
> +        delims (list): Optional, list of delimiters of the list. Defaults to
> +            [',']
> +    """
> +
> +    def __init__(self, decoders=None, delims=None):

Guess delims=[","] would reflect the default value (and saves the below definition).

> +        self._decoders = decoders or ListDecoders()
> +        self._keyval = list()
> +        delims = delims or [","]
> +        delims.append("$")
> +        self._regexp = r"({})".format("|".join(delims))
> +
> +    def kv(self):
> +        return self._keyval
> +
> +    def __iter__(self):
> +        return iter(self._keyval)
> +
> +    def parse(self, string):
> +        """Parse the list in string
> +
> +        Args:
> +            string (str): the string to parse
> +
> +        Raises:
> +            ParseError if any parsing error occurs.
> +        """
> +        kpos = 0
> +        index = 0
> +        while kpos < len(string) and string[kpos] != "\n":
> +            split_parts = re.split(self._regexp, string[kpos:], 1)
> +            if len(split_parts) < 3:

It’s late Friday, so I might miss the reason, but why < 3?

> +                break
> +
> +            value_str = split_parts[0]
> +
> +            key, value = self._decoders.decode(index, value_str)
> +
> +            meta = KeyMetadata(
> +                kpos=kpos,
> +                vpos=kpos,
> +                kstring=value_str,
> +                vstring=value_str,
> +            )
> +            self._keyval.append(KeyValue(key, value, meta))
> +
> +            kpos += len(value_str) + 1
> +            index += 1
> +
> +
> +def decode_nested_list(decoders, delims, value):
> +    """Extracts nested list from te string and returns it in a dictionary

"from the string...”

> +    them in a dictionary

Sentence makes no sense;  it in a dictionary them in a dictionary

> +
> +    Args:
> +        decoders (ListDecoders): the ListDecoders to use.
> +        value (str): the value string to decode.
> +    """
> +    parser = ListParser(decoders, delims)
> +    parser.parse(value)
> +    return {kv.key: kv.value for kv in parser.kv()}
> +
> +
> +def nested_list_decoder(decoders=None, delims=None):
> +    """Helper function that creates a nested list decoder with given
> +    ListDecoders
> +    """
> +    return functools.partial(decode_nested_list, decoders, delims)
> -- 
> 2.31.1

On 12/17/21 14:41, Eelco Chaudron wrote:
> See some minor comment below.
> 
> //Eelco
> 
> 
> On 22 Nov 2021, at 12:22, Adrian Moreno wrote:
> 
>> Some openflow or dpif flows encode their arguments in lists, eg:
>> "some_action(arg1,arg2,arg3)". In order to decode this in a way that can
>> be then stored and queried, add ListParser and ListDecoders classes
>> that parse lists into KeyValue instances.
>>
>> The ListParser / ListDecoders mechanism is quite similar to KVParser and
>> KVDecoders. Since the "key" of the different KeyValue objects is now
>> ommited, it has to be provided by ListDecoders.
>>
>> For example, take the openflow action "resubmit" that can be written as:
>>
>> resubmit([port],[table][,ct])
>>
>> Can be decoded by creating a ListDecoders instance such as:
>>
>> ListDecoders([
>>                  ("port", decode_default),
>>                  ("table", decode_int),
>>                  ("ct", decode_flag),
>>              ])
>>
>> Naturally, the order of the decoders must be kept.
>>
>> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
>> ---
>>   python/automake.mk       |   3 +-
>>   python/ovs/flows/list.py | 123 +++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 125 insertions(+), 1 deletion(-)
>>   create mode 100644 python/ovs/flows/list.py
>>
>> diff --git a/python/automake.mk b/python/automake.mk
>> index 13aa2b4c3..a3265292d 100644
>> --- a/python/automake.mk
>> +++ b/python/automake.mk
>> @@ -44,7 +44,8 @@ ovs_pyfiles = \
>>   	python/ovs/winutils.py \
>>   	python/ovs/flows/__init__.py \
>>   	python/ovs/flows/decoders.py \
>> -	python/ovs/flows/kv.py
>> +	python/ovs/flows/kv.py \
>> +	python/ovs/flows/list.py
>>
>>   # These python files are used at build time but not runtime,
>>   # so they are not installed.
>> diff --git a/python/ovs/flows/list.py b/python/ovs/flows/list.py
>> new file mode 100644
>> index 000000000..d7ad315a6
>> --- /dev/null
>> +++ b/python/ovs/flows/list.py
>> @@ -0,0 +1,123 @@
>> +import re
>> +import functools
>> +
>> +from ovs.flows.kv import KeyValue, KeyMetadata, ParseError
>> +from ovs.flows.decoders import decode_default
>> +
>> +
>> +class ListDecoders:
>> +    """ListDecoders is used by ListParser to decode the elements in the list
> 
> Please add dots here, and to the other comments.
> 
>> +    A decoder is a function that accepts a value and returns its decoded
>> +    object
>> +    The list_decoder to be used is determined by index in the list provided to
>> +    ListDecoders is important.
> 
> This last sentence does not make sense.
> 

Agree :)

>> +
>> +    Args:
>> +        decoders (list of tuples): Optional,  A list of tuples.
>> +            The first element in the tuple is the keyword associated with the
>> +            value. The second element in the tuple is the decoder function.
>> +    """
>> +
>> +    def __init__(self, decoders=None):
>> +        self._decoders = decoders or list()
>> +
>> +    def decode(self, index, value_str):
>> +        """Decode the index'th element of the list
>> +
>> +        Args:
>> +            index (int): the position in the list of the element ot decode
> 
> Guess ot, should be to.
> 
>> +            value_str (str): the value string to decode
>> +        """
>> +        if index < 0 or index >= len(self._decoders):
>> +            return self._default_decoder(index, value_str)
>> +
>> +        try:
>> +            key = self._decoders[index][0]
>> +            value = self._decoders[index][1](value_str)
>> +            return key, value
>> +        except Exception as e:
>> +            raise ParseError(
>> +                "Failed to decode value_str %s: %s" % (value_str, str(e))
> 
> Personally, I try to move away from % formatting and use .format() but not sure what the general rule for OVS is.
> I know there now also is f””, but I’m not using that yet.
> 

I normally also prefer .format(), don't know why this slipped. Thanks for 
spotting it.

>> +            )
>> +
>> +    @staticmethod
>> +    def _default_decoder(index, value):
>> +        key = "elem_{}".format(index)
>> +        return key, decode_default(value)
>> +
>> +
>> +class ListParser:
>> +    """ListParser parses a list of values and stores them as key-value pairs
>> +
>> +    It uses a ListDecoders instance to decode each element in the list.
>> +
>> +    Args:
>> +        decoders (ListDecoders): Optional, the decoders to use
>> +        delims (list): Optional, list of delimiters of the list. Defaults to
>> +            [',']
>> +    """
>> +
>> +    def __init__(self, decoders=None, delims=None):
> 
> Guess delims=[","] would reflect the default value (and saves the below definition).
> 

Will change it in the next version.

>> +        self._decoders = decoders or ListDecoders()
>> +        self._keyval = list()
>> +        delims = delims or [","]
>> +        delims.append("$")
>> +        self._regexp = r"({})".format("|".join(delims))
>> +
>> +    def kv(self):
>> +        return self._keyval
>> +
>> +    def __iter__(self):
>> +        return iter(self._keyval)
>> +
>> +    def parse(self, string):
>> +        """Parse the list in string
>> +
>> +        Args:
>> +            string (str): the string to parse
>> +
>> +        Raises:
>> +            ParseError if any parsing error occurs.
>> +        """
>> +        kpos = 0
>> +        index = 0
>> +        while kpos < len(string) and string[kpos] != "\n":
>> +            split_parts = re.split(self._regexp, string[kpos:], 1)
>> +            if len(split_parts) < 3:
> 
> It’s late Friday, so I might miss the reason, but why < 3?

This is actually a bug that I'll fix in the next revision.

Normally, re.split returns a list of 3 elements
[preface, delimiter, rest]

In order to easily get the last item of the list, a possibility is to add "$" to 
the (end of line) delimiter regexp. However, python 3.6 and 3.7 have different 
return different things when the match is the end of string:

python 3.6 returns a one element list: [preface]
python 3.7 returns a 3 element list [preface, "", ""]

In the next revision I'll change this part to just check if "len(split_parts) == 
0". This means there are no more elements to process and works for both python 
versions.


> 
>> +                break
>> +
>> +            value_str = split_parts[0]
>> +
>> +            key, value = self._decoders.decode(index, value_str)
>> +
>> +            meta = KeyMetadata(
>> +                kpos=kpos,
>> +                vpos=kpos,
>> +                kstring=value_str,
>> +                vstring=value_str,
>> +            )
>> +            self._keyval.append(KeyValue(key, value, meta))
>> +
>> +            kpos += len(value_str) + 1
>> +            index += 1
>> +
>> +
>> +def decode_nested_list(decoders, delims, value):
>> +    """Extracts nested list from te string and returns it in a dictionary
> 
> "from the string...”
> 
>> +    them in a dictionary
> 
> Sentence makes no sense;  it in a dictionary them in a dictionary
> 
>> +
>> +    Args:
>> +        decoders (ListDecoders): the ListDecoders to use.
>> +        value (str): the value string to decode.
>> +    """
>> +    parser = ListParser(decoders, delims)
>> +    parser.parse(value)
>> +    return {kv.key: kv.value for kv in parser.kv()}
>> +
>> +
>> +def nested_list_decoder(decoders=None, delims=None):
>> +    """Helper function that creates a nested list decoder with given
>> +    ListDecoders
>> +    """
>> +    return functools.partial(decode_nested_list, decoders, delims)
>> -- 
>> 2.31.1
>