On 6/6/25 11:49 PM, Terry Wilson via dev wrote:
> Row stringification happens a lot in client logs and it is far
> more useful to have the logged Row's uuid printed. This also
> adds converting referenced Row objects, and references within
> set and map columns to UUIDs.
>
> Signed-off-by: Terry Wilson <twil...@redhat.com>
> ---
> python/ovs/db/idl.py | 32 ++++++++++++++++++++++++++++----
> tests/test-ovsdb.py | 3 +++
> 2 files changed, 31 insertions(+), 4 deletions(-)
Hi, Terry. Thanks for the patch!
This looks like a goos improvement indeed, printing out pointers
instead of map content is indeed not user friendly.
I think, there is some part missing though, I tried to print out
the _Server database row and got this:
Database(uuid=<1> cid=[] connected=true index=[] leader=true model=standalone
name=_Server schema=[{"cksum":"3009684573
744""name":"_Server""tables":{"Database":{"columns":{"cid":{"type":{"key":"uuid""min":0}}"connected":{"type":"boolean"}"index":{"type":{"key":"integer""min":0}}"leader":{"type":"boolean"}"model":{"type":{"key":{"enum":["set"["clustered""relay""standalone"]]"type":"string"}}}"name":{"type":"string"}"schema":{"type":{"key":"string""min":0}}"sid":{"type":{"key":"uuid""min":0}}}}}"version":"1.2.0"}]
sid=[])
It all looks good, but there is no delimiter between map elements,
e.g. "cksum":"3009684573 744""name":"_Server". I'd expect a space
or a comma between the entries. Can this be done?
A couple nits below.
>
> diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py
> index f01e21b5e..4188d1842 100644
> --- a/python/ovs/db/idl.py
> +++ b/python/ovs/db/idl.py
> @@ -1229,6 +1229,29 @@ def _row_to_uuid(value):
> return value
>
>
> +def _rows_to_uuid_str(value):
> + if isinstance(value, collections.abc.Mapping):
> + try:
> + k, v = next(iter(value.items()))
> + # Pass through early without iterating if not Rows
Period at the end of a comment.
> + if isinstance(k, Row) or isinstance(v, Row):
> + return {str(_row_to_uuid(x)): str(_row_to_uuid(y))
> + for x, y in value.items()}
> + except StopIteration:
> + # Empty, return default
ditto.
> + pass
> + elif (isinstance(value, collections.abc.Iterable)
> + and not isinstance(value, str)):
> + try:
> + # Pass through early without iterating if not Rows
ditto.
> + if value and isinstance(value[0], Row):
> + return type(value)(str(_row_to_uuid(x)) for x in value)
> + except TypeError:
> + # Weird Iterable, pass through
ditto.
> + pass
> + return str(_row_to_uuid(value))
> +
> +
> @functools.total_ordering
> class Row(object):
> """A row within an IDL.
> @@ -1334,11 +1357,12 @@ class Row(object):
> return int(self.__dict__['uuid'])
>
> def __str__(self):
> - return "{table}({data})".format(
> + return "{table}(uuid={uuid}, {data})".format(
> table=self._table.name,
> - data=", ".join("{col}={val}".format(col=c, val=getattr(self, c))
> - for c in sorted(self._table.columns)
> - if hasattr(self, c)))
> + uuid=self.uuid,
> + data=", ".join("{col}={val}".format(
> + col=c, val=_rows_to_uuid_str(getattr(self, c)))
> + for c in sorted(self._table.columns) if hasattr(self, c)))
>
> def _uuid_to_row(self, atom, base):
> if base.ref_table:
> diff --git a/tests/test-ovsdb.py b/tests/test-ovsdb.py
> index b690b4f07..32e1894e1 100644
> --- a/tests/test-ovsdb.py
> +++ b/tests/test-ovsdb.py
> @@ -214,6 +214,9 @@ def do_parse_schema(schema_string):
>
>
> def get_simple_printable_row_string(row, columns):
> + # NOTE(twilson):This turns out to be a particularly good place to test
> that
> + # Row object stringification doesn't crash on a large variety of row
> types
> + assert(str(row))
flake8 complains that this should be 'assert str(row)' instead.
It would be nice if we could just use the result here instead of
table-specific printing functions, but that will require a large
amount of test changes, so it may be good to do sometime later.
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
