Re: [PATCH net-next v3 2/6] tools/net/ynl: Report netlink errors without stacktrace

On Thu, Mar 07, 2024 at 07:58:15AM -0800, Jakub Kicinski wrote:

On Thu, 7 Mar 2024 11:56:59 +0000 Donald Hunter wrote:

quoted

quoted

Basically this is just hidding the stack, which may make it harder for
someone not used to the code to find the problem.

Usually fatal exception is handled to make the error more meaningful,
i.e, better than just the exception message + stack. Hidding the stack
and exitting may make the error less meaningful.  

NlError is used to report a usage error reported by netlink as opposed
to a fatal exception. My thinking here is that it is better UX to
report netlink error responses without the stack trace precisely
because they are not exceptional. An NlError is not ynl program
breakage or subsystem breakage, it's e.g. nlctrl telling you that you
requested an op that does not exist.

Right, I think the YNL library should still throw, but since this is
a case of "kernel gave us this specific error in response" the stack
trace adds relatively little for the CLI.

quoted

quoted

On a different topic, I am wondering if we want to add type hitting for
these python program. They make the review process easier, and the
development a bit more structured. (Maybe that is what we expect from
upcoming new python code in netdev?!)  

It's a good suggestion. I have never used python type hints so I'll
need to learn about them. I defer to the netdev maintainers about
whether this is something they want.

I'm far from a Python expert, so up to you :)
I used type hints a couple of times in the past, they are somewhat
useful, but didn't feel useful enough to bother. Happy for someone
else to do the work, tho :)

I am a big fan of type hitting, since it help in reviewing code, as also
with tooling that help you to find problems, since the function returns
and arguments now have a type.

What are the top 3 python scripts we have in network today? I can try to
find some time to help.

FWIW I reckon that trying to get the CLI ready for distro packaging 
may be higher prio. Apart from basic requirements to packaging python
code (I have no idea what they are), we should probably extend the
script to search some system paths? My thinking is that if someone
installs the CLI as an RPM, they should be able to use it like this:

 $ ynl-cli --family nlctrl \
	--do getfamily --json '{"family-name": "nlctrl"}'

the --family would be used instead of --spec and look for the exact
spec file in /usr/share/.../specs/ and probably also imply --no-schema,
since hopefully the schema is already validated during development,
and no point wasting time validating it on every user invocation.

WDYT?

This is a good idea. I've had a chat with Michel, and he can help with
the distro part. Adding him in the CC.