Re: [PATCH 09/13] iotests: split linters.py out from 297

[Hanna Reitz]

On 13.10.21 17:07, John Snow wrote:
> On Wed, Oct 13, 2021 at 7:50 AM Hanna Reitz <hreitz@redhat.com> wrote:
>
>     On 04.10.21 23:04, John Snow wrote:
>     > Now, 297 is just the iotests-specific incantations and
>     linters.py is as
>     > minimal as I can think to make it. The only remaining element in
>     here
>     > that ought to be configuration and not code is the list of skip
>     files,
>
>     Yeah...
>
>     > but they're still numerous enough that repeating them for mypy and
>     > pylint configurations both would be ... a hassle.
>
>     I agree.
>
>     > Signed-off-by: John Snow <jsnow@redhat.com>
>     > ---
>     >   tests/qemu-iotests/297        | 72 +++---------------------------
>     >   tests/qemu-iotests/linters.py | 83
>     +++++++++++++++++++++++++++++++++++
>     >   2 files changed, 88 insertions(+), 67 deletions(-)
>     >   create mode 100644 tests/qemu-iotests/linters.py
>
>     I’d like to give an A-b because now the statuscode-returning
>     function is
>     in a library.  But I already gave an A-b on the last patch precisely
>     because of the interface, and I shouldn’t be so grumpy.
>
>     Reviewed-by: Hanna Reitz <hreitz@redhat.com>
>
>
> I'm not entirely sure I understand your dislike(?) of status codes. 
> I'm not trying to ignore the feedback, but I don't think I understand 
> it fully.

It’s the fact that we only use status codes because they are part of the 
interface of shell commands.  A python function isn’t a shell command, 
so I find it weird to use that interface there.  Returning True/False 
would make more sense, for example.

I understand we have the same thing with qemu* commands in iotests.py, 
so I shouldn’t be so stuck on it...

> Would it be better if I removed check=False and allowed the functions 
> to raise an Exception on non-zero subprocess return? (Possibly having 
> the function itself print the stdout on the error case before re-raising.)

Yes, I would like that better! :)

Hanna