netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Donald Hunter <donald.hunter@gmail.com>,
	Breno Leitao <leitao@debian.org>
Cc: netdev@vger.kernel.org, "David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Paolo Abeni <pabeni@redhat.com>,
	Jacob Keller <jacob.e.keller@intel.com>,
	Jiri Pirko <jiri@resnulli.us>,
	Stanislav Fomichev <sdf@google.com>,
	donald.hunter@redhat.com
Subject: Re: [PATCH net-next v3 2/6] tools/net/ynl: Report netlink errors without stacktrace
Date: Thu, 7 Mar 2024 07:58:15 -0800	[thread overview]
Message-ID: <20240307075815.19641934@kernel.org> (raw)
In-Reply-To: <CAD4GDZyvvSPV_-nZsB1rUb1wK6i-Z_DuK=PPLP4BTnfC1CLz3Q@mail.gmail.com>

On Thu, 7 Mar 2024 11:56:59 +0000 Donald Hunter wrote:
> > 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.

> > 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 :)

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?

  reply	other threads:[~2024-03-07 15:58 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-06 23:10 [PATCH net-next v3 0/6] tools/net/ynl: Add support for nlctrl netlink family Donald Hunter
2024-03-06 23:10 ` [PATCH net-next v3 1/6] tools/net/ynl: Fix extack decoding for netlink-raw Donald Hunter
2024-03-06 23:10 ` [PATCH net-next v3 2/6] tools/net/ynl: Report netlink errors without stacktrace Donald Hunter
2024-03-07  9:31   ` Breno Leitao
2024-03-07 11:56     ` Donald Hunter
2024-03-07 15:58       ` Jakub Kicinski [this message]
2024-03-08 18:27         ` Breno Leitao
2024-03-08 19:16           ` Jakub Kicinski
2024-03-06 23:10 ` [PATCH net-next v3 3/6] tools/net/ynl: Fix c codegen for array-nest Donald Hunter
2024-03-06 23:10 ` [PATCH net-next v3 4/6] tools/net/ynl: Add nest-type-value decoding Donald Hunter
2024-03-06 23:10 ` [PATCH net-next v3 5/6] doc/netlink: Allow empty enum-name in ynl specs Donald Hunter
2024-03-07  2:26   ` Jakub Kicinski
2024-03-07  9:08     ` Donald Hunter
2024-03-07 15:21       ` Jakub Kicinski
2024-03-06 23:10 ` [PATCH net-next v3 6/6] doc/netlink/specs: Add spec for nlctrl netlink family Donald Hunter
2024-03-08  5:00 ` [PATCH net-next v3 0/6] tools/net/ynl: Add support " patchwork-bot+netdevbpf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240307075815.19641934@kernel.org \
    --to=kuba@kernel.org \
    --cc=davem@davemloft.net \
    --cc=donald.hunter@gmail.com \
    --cc=donald.hunter@redhat.com \
    --cc=edumazet@google.com \
    --cc=jacob.e.keller@intel.com \
    --cc=jiri@resnulli.us \
    --cc=leitao@debian.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=sdf@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).