On Fri, 8 Oct 2021, Leah Leshchinsky wrote:
On Wed, Oct 06, 2021 at 02:39:47PM -0400, John Kacur wrote:
>
>
> On Wed, 6 Oct 2021, Leah Leshchinsky wrote:
>
> > Currently, the command `tuna --cpus=CPU-LIST --spread` generates an
> > error with the warning "tuna: --move requires a list of
threads/irqs!".
> > Similarly, when the command `tuna --threads=THREAD-LIST --spread` is
> > run, it generates the warning "tuna: --move requires a cpu list!".
> >
> > This can be confusing to the user, especially with commands that use both
> > the "--spread" and "--move" flags. The warning should
specify "--spread"
> > when that is the source of the error.
> >
> > Check whether the argument is a move or spread and update the warning
> > string accordingly.
> >
> > Signed-off-by: Leah Leshchinsky <lleshchi(a)redhat.com>
> > ---
> > tuna-cmd.py | 7 +++----
> > 1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/tuna-cmd.py b/tuna-cmd.py
> > index 8617966..8dfad9e 100755
> > --- a/tuna-cmd.py
> > +++ b/tuna-cmd.py
> > @@ -633,15 +633,14 @@ def main():
> > elif o in ("-n", "--show_sockets"):
> > show_sockets = True
> > elif o in ("-m", "--move", "-x",
"--spread"):
> > + spread = o in ("-x", "--spread")
> > if not cpu_list:
> > - print("tuna: --move " + _("requires a cpu
list!"))
> > + print("tuna: %s " % ("--spread" if spread
else "--move") + _("requires a cpu list!"))
> > sys.exit(2)
> > if not (thread_list or irq_list):
> > - print("tuna: --move " + _("requires a list of
threads/irqs!"))
> > + print("tuna: %s " % ("--spread" if spread
else "--move") + _("requires a list of threads/irqs!"))
> > sys.exit(2)
> >
> > - spread = o in ("-x", "--spread")
> > -
> > if thread_list:
> > tuna.move_threads_to_cpu(cpu_list, thread_list,
spread=spread)
> >
> > --
>
>
> Excellent catch.
>
> You could make this even simpler too though. It would also be confusing if
> the user typed '-x' and the message says '--spread', but since the
> variable 'o' contains exactly what the user passed you could print that
> directly without even moving the spread = o in ("-x",
"--spread") line.
>
> Eg, change the first print to print(f"tuna: {o} requires a cpu list!")
> and do something similar with the second print
>
> John
In my opinion, if we change the warning message to '-x' and '-m' when
the short version of the '--spread' and '--move' arguments are called,
we should apply this standard to all warning messages.
At the moment, tuna prints the long option form of any argument in its
warning messages.
-Leah
You're right, it would be inconsistent if we just do that in one place.
Maybe the long form makes sense, since when you read it you know what it
means, and the short version is an abbreviation for that anyway.
Applying your original patch for now, we can consider if we want to change
that in the future.
Signed-off-by: John Kacur <jkacur(a)redhat.com>