Re: [PATCH net-next] tools: ynl: rework the string representation of NlError

[Donald Hunter <donald.hunter@gmail.com>]

Jakub Kicinski <kuba@kernel.org> writes:

> In early days of YNL development dumping the NlMsg on errors
> was quite useful, as the library itself could have been buggy.
> These days increasingly the NlMsg is just taking up screen space
> and means nothing to a typical user. Try to format the errors
> more in line with how YNL C formats its errors strings.
>
> Before:
>   $ ynl --family ethtool  --do channels-set  --json '{}'
>   Netlink error: Invalid argument
>   nl_len = 44 (28) nl_flags = 0x300 nl_type = 2
> 	error: -22
> 	extack: {'miss-type': 'header'}
>
>   $ ynl --family ethtool  --do channels-set  --json '{..., "tx-count": 999}'
>   Netlink error: Invalid argument
>   nl_len = 88 (72) nl_flags = 0x300 nl_type = 2
> 	error: -22
> 	extack: {'msg': 'requested channel count exceeds maximum', 'bad-attr': '.tx-count'}
>
> After:
>   $ ynl --family ethtool  --do channels-set  --json '{}'
>   Netlink error: Invalid argument {'miss-type': 'header'}
>
>   $ ynl --family ethtool  --do channels-set  --json '{..., "tx-count": 999}'
>   Netlink error: requested channel count exceeds maximum: Invalid argument {'bad-attr': '.tx-count'}
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> CC: donald.hunter@gmail.com
> CC: ast@fiberby.net
> ---
>  tools/net/ynl/pyynl/lib/ynl.py | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/tools/net/ynl/pyynl/lib/ynl.py b/tools/net/ynl/pyynl/lib/ynl.py
> index 62383c70ebb9..bac9eb33ba89 100644
> --- a/tools/net/ynl/pyynl/lib/ynl.py
> +++ b/tools/net/ynl/pyynl/lib/ynl.py
> @@ -105,7 +105,17 @@ from .nlspec import SpecFamily
>      self.error = -nl_msg.error
>  
>    def __str__(self):
> -    return f"Netlink error: {os.strerror(self.error)}\n{self.nl_msg}"
> +    msg = "Netlink error: "
> +
> +    extack = self.nl_msg.extack.copy() if self.nl_msg.extack else {}
> +    if extack:

The 'if extack' condition seems redundant given the way extack is
initialised.

Otherwise LGTM.

> +        if 'msg' in extack:
> +            msg += extack['msg'] + ': '
> +            del extack['msg']
> +    msg += os.strerror(self.error)
> +    if extack:
> +        msg += ' ' + str(extack)
> +    return msg
>  
>  
>  class ConfigError(Exception):