On Thu, Mar 17, 2022, 6:53 AM Hanna Reitz <hre...@redhat.com> wrote:
> On 09.03.22 04:53, John Snow wrote:
> > qemu_img_json() is a new helper built on top of qemu_img() that tries to
> > pull a valid JSON document out of the stdout stream.
> >
> > In the event that the return code is negative (the program crashed), or
> > the code is greater than zero and did not produce valid JSON output, the
> > VerboseProcessError raised by qemu_img() is re-raised.
> >
> > In the event that the return code is zero but we can't parse valid JSON,
> > allow the JSON deserialization error to be raised.
> >
> > Signed-off-by: John Snow <js...@redhat.com>
> > ---
> > tests/qemu-iotests/iotests.py | 38 +++++++++++++++++++++++++++++++++++
> > 1 file changed, 38 insertions(+)
> >
> > diff --git a/tests/qemu-iotests/iotests.py
> b/tests/qemu-iotests/iotests.py
> > index 7057db0686..546b142a6c 100644
> > --- a/tests/qemu-iotests/iotests.py
> > +++ b/tests/qemu-iotests/iotests.py
> > @@ -276,6 +276,44 @@ def ordered_qmp(qmsg, conv_keys=True):
> > def qemu_img_create(*args: str) -> subprocess.CompletedProcess[str]:
> > return qemu_img('create', *args)
> >
> > +def qemu_img_json(*args: str) -> Any:
> > + """
> > + Run qemu-img and return its output as deserialized JSON.
> > +
> > + :raise CalledProcessError:
> > + When qemu-img crashes, or returns a non-zero exit code without
> > + producing a valid JSON document to stdout.
> > + :raise JSONDecoderError:
> > + When qemu-img returns 0, but failed to produce a valid JSON
> document.
> > +
> > + :return: A deserialized JSON object; probably a dict[str, Any].
> > + """
> > + json_data = ... # json.loads can legitimately return 'None'.
>
> What kind of arcane sigil is this?
>
I may genuinely start referring to it as the Arcane Sigil.
> I’m trying to read into it, but it seems like... Well, I won’t swear on
> a public list. As far as I understand, it’s just a special singleton
> object that you can use for whatever you think is an appropriate use for
> an ellipsis? And so in this case you use it as a special singleton
> object that would never legitimately appear, to be separate from None?
>
> You’re really, really good at making me hate Python a bit more with
> every series.
>
I aim to please.
Yes, it's just Another Singleton That Isn't None, because technically a
JSON document can be just "null". Will qemu_img() ever, ever print that
single string all by itself?
Well, I hope not, but. I felt guilty not addressing that technicality
somehow.
> It also just doesn’t seem very useful to me in this case. I’m not sure
> whether this notation is widely known in the Python world, but I have
> only myself to go off of, and I was just very confused, so I would
> prefer not to have this in the code.
>
You're right, it's a bit too clever. I judged the cleverness:usefulness
ratio poorly.
(I think it's a trick that a lot of long time python people know, because
sooner or later one wants to distinguish between an explicitly provided
"None" and a default value that signifies "No argument provided". It's
definitely a hack. It's not something most users would know.)
> > +
> > + try:
> > + res = qemu_img(*args, combine_stdio=False)
> > + except subprocess.CalledProcessError as exc:
> > + # Terminated due to signal. Don't bother.
> > + if exc.returncode < 0:
> > + raise
> > +
> > + # Commands like 'check' can return failure (exit codes 2 and 3)
> > + # to indicate command completion, but with errors found. For
> > + # multi-command flexibility, ignore the exact error codes and
> > + # *try* to load JSON.
> > + try:
> > + json_data = json.loads(exc.stdout)
>
> Why not `return json.loads(exc.stdout)`?
>
Uh.
> > + except json.JSONDecodeError:
> > + # Nope. This thing is toast. Raise the process error.
> > + pass
> > +
> > + if json_data is ...:
> > + raise
>
> And just unconditionally `raise` here.
Uhhh.
> > +
> > + if json_data is ...:
> > + json_data = json.loads(res.stdout)
> > + return json_data
>
> And just `return json.loads(res.stdout)` here, without any condition.
>
Uhhhhhhhh...!
Yeah, that's obviously way better. I'm sorry to have subjected you to an
arcane workaround for no reason :/
>
> Hanna
>
> > +
> > def qemu_img_measure(*args):
> > return json.loads(qemu_img_pipe("measure", "--output", "json",
> *args))
> >
>
>
