Notes on Generating Python signatures for QMP RPCs

Notes on Generating Python signatures for QMP RPCs

[John Snow]

Hiya, I was experimenting with $subject and ran into a few points of
interest. This is basically an informal status report from me. I've
CC'd some of the usual suspects for people who care about SDKs and API
design and such.

This is just a list of some observations I had, so not everything
below is a question or an action item. Just sharing some notes.

(0) This experiment concerned generating signatures based on
introspection data, dynamically at runtime. In this environment type
hints are not required, as they are not actually used at runtime.
However, I added them anyway as an exercise for dynamic documentation
purposes. (i.e. `help(auto_generated_function)` showing type hints can
still be useful -- especially without access to QAPI doc blocks.)
Determining type information is also necessary for generating the
marshaling/unmarshaling functions to communicate with the server.

(1) QAPI types the return of many commands as an empty object. That's
literally indeed what happens on the wire, and it makes sense in that
if these commands were ever to return anything, it is a "compatible
evolution" to include new fields in such an object. In Python, this
does not make much sense, though; as this is somewhat hard to
annotate:

async def stop() -> Literal[{}]: ...

The more pythonic signature is:

async def stop() -> None: ...

I feel like it's spiritually equivalent, but I am aware it is a
distinct choice that is being made. It could theoretically interfere
with a choice made in QAPI later to explicitly return Null. I don't
think we'd do that, but it's still a choice of abstraction that
reduces the resolution of distinct return signatures.

(1.5) Do we have a formal definition for what we consider to be a
"compatible evolution" of the schema? I've got a fairly good idea, but
I am not sure how it's enforced. Is it just Markus being very
thorough? If we add more languages to the generator, we probably can't
burden Markus with knowing how to protect the compatibility of every
generator. We might need more assertions for invariants in the
generator itself ... but to protect "evolution", we need points of
reference to test against. Do we have anything for this? Do we need
one? Should I write a test?

(2) There are five commands that are exempted from returning an
object. qom-get is one. However, what I didn't really explicitly
realize is that this doesn't mean that only five commands don't return
an object -- we also actually allow for a list of objects, which
*many* commands use. There's no technical issue here, just an
observation. It is no problem at all to annotate Python commands as
"-> SomeReturnType" or "-> List[SomeDifferentReturnType]" or even "->
str:" as needed.

(3) Over the wire, the order of arguments to QMP commands is not
specified. In generating commands procedurally from introspection
data, I am made aware that there are several commands in which
"optional" arguments precede "required" arguments. This means that
function generation in Python cannot match the stated order 1:1.

That's not a crisis per se. For generating functions, we can use a
stable sort to bubble-up the required arguments, leaving the optional
ones trailing. However, it does mean that depending on how the QAPI
schema is modified in the future, the argument order may change
between versions of a generative SDK. I'd like to avoid that, if I
can.

One trick I have available to me in Python is the ability to stipulate
that all (QAPI) "optional" arguments are keyword-only. This means that
Optional parameters can be re-ordered arbitrarily without any breakage
in the generative python API. The only remaining concern is if the
*mandatory* arguments are re-ordered.

(In fact, I could stipulate that ALL arguments in Python bindings are
keyword-only, but I think that's going overboard and hurts usability
and readability.)

Marc-Andre has mentioned this before, but it might be nice to actually
specify a canonical ordering of arguments for languages that require
such things, and to make sure that we do not break this ordering
without good reason.

(Of course, SDK stability is not fully possible, and if this
functionality is desired, then it's time to use libvirt, hint hint
hint! However, we can avoid pointless churn in generated code and make
it easier to use and experiment with.)

(4) StrOrNull is a tricky design problem.

In Python, generally, omitted arguments are typed like this:
async def example_command(arg: Optional[int] = None) -> None: ...

Most Python programmers would recognize that signature as meaning that
they can omit 'arg' and some hopefully good default will be chosen.
However, in QAPI we do have the case where "omitted" is distinct from
"explicitly provided null". This is ... a bit challenging to convey
semantically. Python does not offer the ability to tell "what kind of
None" it received; i.e. unlike our generated QMP marshalling
functions, we do not have a "has_arg" boolean we can inspect.

So how do we write a function signature that conveys the difference
between "omitted" and "explicitly nulled" ...?

One common trick in Python is to create a new sentinel singleton, and
name it something like "Default" or "Unspecified" or "Undefined". Many
programmers use the ellipsis `...` value for this purpose. Then, we
can check if a value was omitted (`...`) or explicitly provided
(`None`). It is very unlikely that these sentinels would organically
collide with user-provided values (Unless they were trying to
explicitly invoke default behavior.)

However, `...` isn't supported as a type and using it as the default
value invalidates the typing of the field. As far as I can tell, it
CANNOT be typed. We could create our own sentinel, but IMO, this
creates a much less readable signature:

async def example_command(arg: Union[int, qmp.Default] = qmp.Default)
-> None: ...

This probably doesn't communicate "This parameter is actually
optional" to a casual Python programmer, so I think it's a dead end.

The last thing I can think of here is to instead introduce a special
sentinel that represents the explicit Null instead. We could use a
special Null() type that means "Explicitly send a null over the wire."

This value comes up fairly infrequently, so most signatures will
appear "Pythonic" and the jankiness will be confined to the few
commands that require it, e.g.

async def example_command(arg: Optional[Union[int, Null]] = None) -> None: ...

The above would imply an optional argument that can be omitted, can be
provided with an int, or can be provided with an explicit Null. I
think this is a good compromise.

(5) Generating functions from introspection data is difficult because
all of the structures are anonymous. The base type for most objects
becomes `Dict[str, Any]` but this isn't very descriptive. For Python
3.8+, we can do a little better and use `Dict[Literal["name", "node"],
Any]` to help suggest what keys are valid, but we don't have access to
an in-line definition that pairs key names with values.

Python 3.8+ would allow us the use of TypedDict, but those have to be
generated separately ... AND we still don't have a name for them, so
it'd be a little hogwash to have a function like:

async def some_command(arg: Anon321) -> None: ...

That doesn't really tell me, the human, much of anything. The best
that could perhaps be done is to create type aliases based on the name
of the argument it is the data type for, like "ArgObject". It's a bit
messy. For now, I've just stuck with the boring `Dict[Literal[...],
Any]` definition.

(6) Dealing with variants is hard. I didn't get a working
implementation for them within one day of hacking, so I stubbed them
out. There's no major blocker here, just reporting that I still have
to finish this part of the experiment. I'm pretty happy that Markus
simplified the union types we have, though. To my knowledge, I got
everything else working perfectly.

(7) I have no idea what to do about functions that "may not return".
The QGA stuff in particular, I believe, is prone to some weirdness
that violates the core principles of the QMP spec. Maybe we can add a
"NORETURN" feature flag to those commands in the schema so that
clients can be aware of which commands may break the expectation of
always getting an RPC reply?

(8) Thanks for reading. I'm still buried under my holiday inbox, but I
am trying like hell to catch up on everything. I know I missed a few
calls in which API design was discussed, and I apologize for that.
Please send me invitations using "to: jsnow@redhat.com" to ensure I do
not miss them. I am also frantically trying to clean up the Async QMP
project I was working on to have more mental bandwidth for other
tasks, but it's dragging on a bit longer than I had anticipated.
Please accept my apologies for being somewhat reclusive lately.

I'll (try to) send a status overview of the various projects I'm
working on later to help set priority and discuss with the community
what my goals are and what I'd like to do. I have an awful lot of code
I've built up in local branches that I would like to share, but I'm
already sending code upstream as fast as I can, so maybe I'll just do
an overview at some point and point to unfinished code/experiments so
it's at least not completely unwitnessed work.

I hope 2022 is treating you all well,
--John Snow

Re: Notes on Generating Python signatures for QMP RPCs

[Markus Armbruster]

John Snow <jsnow@redhat.com> writes:

> Hiya, I was experimenting with $subject and ran into a few points of
> interest. This is basically an informal status report from me. I've
> CC'd some of the usual suspects for people who care about SDKs and API
> design and such.
>
> This is just a list of some observations I had, so not everything
> below is a question or an action item. Just sharing some notes.
>
> (0) This experiment concerned generating signatures based on
> introspection data, dynamically at runtime. In this environment type
> hints are not required, as they are not actually used at runtime.
> However, I added them anyway as an exercise for dynamic documentation
> purposes. (i.e. `help(auto_generated_function)` showing type hints can
> still be useful -- especially without access to QAPI doc blocks.)
> Determining type information is also necessary for generating the
> marshaling/unmarshaling functions to communicate with the server.
>
> (1) QAPI types the return of many commands as an empty object. That's
> literally indeed what happens on the wire, and it makes sense in that
> if these commands were ever to return anything, it is a "compatible
> evolution" to include new fields in such an object. In Python, this
> does not make much sense, though; as this is somewhat hard to
> annotate:
>
> async def stop() -> Literal[{}]: ...
>
> The more pythonic signature is:
>
> async def stop() -> None: ...
>
> I feel like it's spiritually equivalent, but I am aware it is a
> distinct choice that is being made. It could theoretically interfere
> with a choice made in QAPI later to explicitly return Null. I don't
> think we'd do that, but it's still a choice of abstraction that
> reduces the resolution of distinct return signatures.

Having functions take any number of arguments and return one result is
traditional in programming, so we hardly notice how odd the asymmetry
actually is.

QAPI commands are symmetric: they take any number of (named) arguments,
and return any number of (named) results.  Can also be viewed as taking
one exactly one argument (always an object) and returning exactly one
value (always an object).

Except some QAPI commands actually return return a list of objects, and
some return a single, unnamed value.  The former are intentional, the
latter are screwups from the early days.

> (1.5) Do we have a formal definition for what we consider to be a
> "compatible evolution" of the schema?

Section "Compatibility considerations" in docs/devel/qapi-code-gen.rst
tries.  I wouldn't call it "a formal definition".

>                                       I've got a fairly good idea, but
> I am not sure how it's enforced. Is it just Markus being very
> thorough?

Yup, with help from other reviewers such as Eric Blake, and enabled by
the relative simplicity of the rules.

>           If we add more languages to the generator, we probably can't
> burden Markus with knowing how to protect the compatibility of every
> generator.

qapi-code-gen.rst applies the compatibility razor to the wire protocol
only.  It doesn't concern itself with language bindings at all.

We've only ever had C bindings.  Designed for internal use only, where
compatible evolution is pretty much irrelevant.

We've talked about bindings for external use, both long ago (C), and
more recently (Rust, Python, ...).

[*] Compatible evolution of language bindings for QAPI is a complex
topic.  Not today.

>            We might need more assertions for invariants in the
> generator itself ... but to protect "evolution", we need points of
> reference to test against. Do we have anything for this? Do we need
> one? Should I write a test?

So far, we don't have software to reason about QAPI schema *changes*.
The generator checks the current QAPI schema, it doesn't consider the
change from the previous version.

A program to analyze a schema change (the difference between two schema
versions) could be useful.  It could also be difficult to write.

> (2) There are five commands that are exempted from returning an
> object. qom-get is one.

This is pragma 'command-returns-exceptions'.  Like its buddy pragmas, it
exists because we started enforcing the rules only after our rule
breaking had calcified in the external interface.

>                         However, what I didn't really explicitly
> realize is that this doesn't mean that only five commands don't return
> an object -- we also actually allow for a list of objects, which
> *many* commands use. There's no technical issue here, just an
> observation. It is no problem at all to annotate Python commands as
> "-> SomeReturnType" or "-> List[SomeDifferentReturnType]" or even "->
> str:" as needed.
>
> (3) Over the wire, the order of arguments to QMP commands is not
> specified.

Correct.

>            In generating commands procedurally from introspection
> data, I am made aware that there are several commands in which
> "optional" arguments precede "required" arguments. This means that
> function generation in Python cannot match the stated order 1:1.
>
> That's not a crisis per se. For generating functions, we can use a
> stable sort to bubble-up the required arguments, leaving the optional
> ones trailing. However, it does mean that depending on how the QAPI
> schema is modified in the future, the argument order may change
> between versions of a generative SDK. I'd like to avoid that, if I
> can.
>
> One trick I have available to me in Python is the ability to stipulate
> that all (QAPI) "optional" arguments are keyword-only. This means that
> Optional parameters can be re-ordered arbitrarily without any breakage
> in the generative python API. The only remaining concern is if the
> *mandatory* arguments are re-ordered.
>
> (In fact, I could stipulate that ALL arguments in Python bindings are
> keyword-only, but I think that's going overboard and hurts usability
> and readability.)

This would match the wire protocol, which only uses keyword, never
positional.  But your usability argument is valid.

> Marc-Andre has mentioned this before, but it might be nice to actually
> specify a canonical ordering of arguments for languages that require
> such things, and to make sure that we do not break this ordering
> without good reason.
>
> (Of course, SDK stability is not fully possible, and if this
> functionality is desired, then it's time to use libvirt, hint hint
> hint! However, we can avoid pointless churn in generated code and make
> it easier to use and experiment with.)

See [*] above.

> (4) StrOrNull is a tricky design problem.
>
> In Python, generally, omitted arguments are typed like this:
> async def example_command(arg: Optional[int] = None) -> None: ...
>
> Most Python programmers would recognize that signature as meaning that
> they can omit 'arg' and some hopefully good default will be chosen.
> However, in QAPI we do have the case where "omitted" is distinct from
> "explicitly provided null". This is ... a bit challenging to convey
> semantically. Python does not offer the ability to tell "what kind of
> None" it received; i.e. unlike our generated QMP marshalling
> functions, we do not have a "has_arg" boolean we can inspect.

Yes.

The QAPI schema language's semantics of optional are at odds with
Python's.

In Python, an absent argument defaults to some value, either one the
programmer provided, or None.  This makes semantics of "absent" obvious
from the signature.

In QAPI, an absent value is distinct from any present value.  Semantics
of "absent" depend the function's *code*, not just its signature.  I
consider this a design mistake.

In many (most?) cases, code treats absent just like one specific present
value.  We could lift this from code to the schema: permit specifying a
default value in the schema, default absent to this value, don't
generate has_FOO.

The remainder could then be handled as the ugly wart it is: have both
has_FOO and FOO in Python, just like in C.  Or handle it using one of
the ideas you describe next.

> So how do we write a function signature that conveys the difference
> between "omitted" and "explicitly nulled" ...?
>
> One common trick in Python is to create a new sentinel singleton, and
> name it something like "Default" or "Unspecified" or "Undefined". Many
> programmers use the ellipsis `...` value for this purpose. Then, we
> can check if a value was omitted (`...`) or explicitly provided
> (`None`). It is very unlikely that these sentinels would organically
> collide with user-provided values (Unless they were trying to
> explicitly invoke default behavior.)
>
> However, `...` isn't supported as a type and using it as the default
> value invalidates the typing of the field. As far as I can tell, it
> CANNOT be typed. We could create our own sentinel, but IMO, this
> creates a much less readable signature:
>
> async def example_command(arg: Union[int, qmp.Default] = qmp.Default)
> -> None: ...
>
> This probably doesn't communicate "This parameter is actually
> optional" to a casual Python programmer, so I think it's a dead end.
>
> The last thing I can think of here is to instead introduce a special
> sentinel that represents the explicit Null instead. We could use a
> special Null() type that means "Explicitly send a null over the wire."
>
> This value comes up fairly infrequently, so most signatures will
> appear "Pythonic" and the jankiness will be confined to the few
> commands that require it, e.g.
>
> async def example_command(arg: Optional[Union[int, Null]] = None) -> None: ...
>
> The above would imply an optional argument that can be omitted, can be
> provided with an int, or can be provided with an explicit Null. I
> think this is a good compromise.

More so, I think, if we manage to substantially reduce "absent is
distinct from any present value".

> (5) Generating functions from introspection data is difficult because
> all of the structures are anonymous.

Introspection is about the wire format.  Type names are not part of it,
and that's why we hide them in introspection.

>                                      The base type for most objects
> becomes `Dict[str, Any]` but this isn't very descriptive. For Python
> 3.8+, we can do a little better and use `Dict[Literal["name", "node"],
> Any]` to help suggest what keys are valid, but we don't have access to
> an in-line definition that pairs key names with values.
>
> Python 3.8+ would allow us the use of TypedDict, but those have to be
> generated separately ... AND we still don't have a name for them, so
> it'd be a little hogwash to have a function like:
>
> async def some_command(arg: Anon321) -> None: ...
>
> That doesn't really tell me, the human, much of anything. The best
> that could perhaps be done is to create type aliases based on the name
> of the argument it is the data type for, like "ArgObject". It's a bit
> messy. For now, I've just stuck with the boring `Dict[Literal[...],
> Any]` definition.

I'm afraid you're trying to press introspection into a role it's not
designed for.

Why not generate from the QAPI schema?

> (6) Dealing with variants is hard. I didn't get a working

Do you mean alternates?

> implementation for them within one day of hacking, so I stubbed them
> out. There's no major blocker here, just reporting that I still have
> to finish this part of the experiment. I'm pretty happy that Markus
> simplified the union types we have, though. To my knowledge, I got
> everything else working perfectly.
>
> (7) I have no idea what to do about functions that "may not return".
> The QGA stuff in particular, I believe, is prone to some weirdness
> that violates the core principles of the QMP spec.

Yes.

docs/interop/qmp-spec.txt dictates a command sends either a success or
an error response.  Makes sense.

QGA has a few commands that shut down the guest.  How could such a
command send a success response?  If it sends it before it initiates
shutdown, response transmission races with shutdown.  The easy way out
is violating qmp-spec.txt.  Thus, 'success-response': false.  Just for
QGA.

>                                                    Maybe we can add a
> "NORETURN" feature flag to those commands in the schema so that
> clients can be aware of which commands may break the expectation of
> always getting an RPC reply?

For a normal command, bindings marshal, send the command, receive the
response, unmarshal.

For a 'success-response': false command, they only marshal and send.

> (8) Thanks for reading. I'm still buried under my holiday inbox, but I
> am trying like hell to catch up on everything. I know I missed a few
> calls in which API design was discussed, and I apologize for that.
> Please send me invitations using "to: jsnow@redhat.com" to ensure I do
> not miss them. I am also frantically trying to clean up the Async QMP
> project I was working on to have more mental bandwidth for other
> tasks, but it's dragging on a bit longer than I had anticipated.
> Please accept my apologies for being somewhat reclusive lately.
>
> I'll (try to) send a status overview of the various projects I'm
> working on later to help set priority and discuss with the community
> what my goals are and what I'd like to do. I have an awful lot of code
> I've built up in local branches that I would like to share, but I'm
> already sending code upstream as fast as I can, so maybe I'll just do
> an overview at some point and point to unfinished code/experiments so
> it's at least not completely unwitnessed work.
>
> I hope 2022 is treating you all well,

Happy new year to you, too!

Re: Notes on Generating Python signatures for QMP RPCs

[John Snow]

On Thu, Jan 27, 2022 at 9:03 AM Markus Armbruster <armbru@redhat.com> wrote:
>
> John Snow <jsnow@redhat.com> writes:
>
> > Hiya, I was experimenting with $subject and ran into a few points of
> > interest. This is basically an informal status report from me. I've
> > CC'd some of the usual suspects for people who care about SDKs and API
> > design and such.
> >
> > This is just a list of some observations I had, so not everything
> > below is a question or an action item. Just sharing some notes.
> >
> > (0) This experiment concerned generating signatures based on
> > introspection data, dynamically at runtime. In this environment type
> > hints are not required, as they are not actually used at runtime.
> > However, I added them anyway as an exercise for dynamic documentation
> > purposes. (i.e. `help(auto_generated_function)` showing type hints can
> > still be useful -- especially without access to QAPI doc blocks.)
> > Determining type information is also necessary for generating the
> > marshaling/unmarshaling functions to communicate with the server.
> >
> > (1) QAPI types the return of many commands as an empty object. That's
> > literally indeed what happens on the wire, and it makes sense in that
> > if these commands were ever to return anything, it is a "compatible
> > evolution" to include new fields in such an object. In Python, this
> > does not make much sense, though; as this is somewhat hard to
> > annotate:
> >
> > async def stop() -> Literal[{}]: ...
> >
> > The more pythonic signature is:
> >
> > async def stop() -> None: ...
> >
> > I feel like it's spiritually equivalent, but I am aware it is a
> > distinct choice that is being made. It could theoretically interfere
> > with a choice made in QAPI later to explicitly return Null. I don't
> > think we'd do that, but it's still a choice of abstraction that
> > reduces the resolution of distinct return signatures.
>
> Having functions take any number of arguments and return one result is
> traditional in programming, so we hardly notice how odd the asymmetry
> actually is.
>
> QAPI commands are symmetric: they take any number of (named) arguments,
> and return any number of (named) results.  Can also be viewed as taking
> one exactly one argument (always an object) and returning exactly one
> value (always an object).
>
> Except some QAPI commands actually return return a list of objects, and
> some return a single, unnamed value.  The former are intentional, the
> latter are screwups from the early days.
>
> > (1.5) Do we have a formal definition for what we consider to be a
> > "compatible evolution" of the schema?
>
> Section "Compatibility considerations" in docs/devel/qapi-code-gen.rst
> tries.  I wouldn't call it "a formal definition".
>
> >                                       I've got a fairly good idea, but
> > I am not sure how it's enforced. Is it just Markus being very
> > thorough?
>
> Yup, with help from other reviewers such as Eric Blake, and enabled by
> the relative simplicity of the rules.
>
> >           If we add more languages to the generator, we probably can't
> > burden Markus with knowing how to protect the compatibility of every
> > generator.
>
> qapi-code-gen.rst applies the compatibility razor to the wire protocol
> only.  It doesn't concern itself with language bindings at all.
>

The generator code certainly does, though. For example, "C name"
collisions are guarded against by the generator -- we do not let it
get as far as blowing up clang.

> We've only ever had C bindings.  Designed for internal use only, where
> compatible evolution is pretty much irrelevant.
>
> We've talked about bindings for external use, both long ago (C), and
> more recently (Rust, Python, ...).
>
> [*] Compatible evolution of language bindings for QAPI is a complex
> topic.  Not today.
>

OK, agreed. It's a big topic. I think it's not fully possible to cover
all cases in all places, but I do seem to recall Marc-Andre having
some opinions about a few things we could do to staple it down just a
little bit, and maybe that's enough. I'd like to talk about it
eventually. Marc-Andre could cover rust, I could cover Python, and
Victor/Andrea could help us cover golang.

> >            We might need more assertions for invariants in the
> > generator itself ... but to protect "evolution", we need points of
> > reference to test against. Do we have anything for this? Do we need
> > one? Should I write a test?
>
> So far, we don't have software to reason about QAPI schema *changes*.
> The generator checks the current QAPI schema, it doesn't consider the
> change from the previous version.
>
> A program to analyze a schema change (the difference between two schema
> versions) could be useful.  It could also be difficult to write.
>

Yeh. Something not worth doing until we chat about what compatible
evolution looks like for other language bindings, because otherwise we
don't know what we're designing for. Later.

> > (2) There are five commands that are exempted from returning an
> > object. qom-get is one.
>
> This is pragma 'command-returns-exceptions'.  Like its buddy pragmas, it
> exists because we started enforcing the rules only after our rule
> breaking had calcified in the external interface.
>
> >                         However, what I didn't really explicitly
> > realize is that this doesn't mean that only five commands don't return
> > an object -- we also actually allow for a list of objects, which
> > *many* commands use. There's no technical issue here, just an
> > observation. It is no problem at all to annotate Python commands as
> > "-> SomeReturnType" or "-> List[SomeDifferentReturnType]" or even "->
> > str:" as needed.
> >
> > (3) Over the wire, the order of arguments to QMP commands is not
> > specified.
>
> Correct.
>
> >            In generating commands procedurally from introspection
> > data, I am made aware that there are several commands in which
> > "optional" arguments precede "required" arguments. This means that
> > function generation in Python cannot match the stated order 1:1.
> >
> > That's not a crisis per se. For generating functions, we can use a
> > stable sort to bubble-up the required arguments, leaving the optional
> > ones trailing. However, it does mean that depending on how the QAPI
> > schema is modified in the future, the argument order may change
> > between versions of a generative SDK. I'd like to avoid that, if I
> > can.
> >
> > One trick I have available to me in Python is the ability to stipulate
> > that all (QAPI) "optional" arguments are keyword-only. This means that
> > Optional parameters can be re-ordered arbitrarily without any breakage
> > in the generative python API. The only remaining concern is if the
> > *mandatory* arguments are re-ordered.
> >
> > (In fact, I could stipulate that ALL arguments in Python bindings are
> > keyword-only, but I think that's going overboard and hurts usability
> > and readability.)
>
> This would match the wire protocol, which only uses keyword, never
> positional.  But your usability argument is valid.
>
> > Marc-Andre has mentioned this before, but it might be nice to actually
> > specify a canonical ordering of arguments for languages that require
> > such things, and to make sure that we do not break this ordering
> > without good reason.
> >
> > (Of course, SDK stability is not fully possible, and if this
> > functionality is desired, then it's time to use libvirt, hint hint
> > hint! However, we can avoid pointless churn in generated code and make
> > it easier to use and experiment with.)
>
> See [*] above.
>
> > (4) StrOrNull is a tricky design problem.
> >
> > In Python, generally, omitted arguments are typed like this:
> > async def example_command(arg: Optional[int] = None) -> None: ...
> >
> > Most Python programmers would recognize that signature as meaning that
> > they can omit 'arg' and some hopefully good default will be chosen.
> > However, in QAPI we do have the case where "omitted" is distinct from
> > "explicitly provided null". This is ... a bit challenging to convey
> > semantically. Python does not offer the ability to tell "what kind of
> > None" it received; i.e. unlike our generated QMP marshalling
> > functions, we do not have a "has_arg" boolean we can inspect.
>
> Yes.
>
> The QAPI schema language's semantics of optional are at odds with
> Python's.
>
> In Python, an absent argument defaults to some value, either one the
> programmer provided, or None.  This makes semantics of "absent" obvious
> from the signature.
>

"None" is never an assumed default in Python, it's always explicitly
assigned as part of the signature. It is just by principle of least
surprise that we all agree to use "None" as that default.

> In QAPI, an absent value is distinct from any present value.  Semantics
> of "absent" depend the function's *code*, not just its signature.  I
> consider this a design mistake.
>

Because of the above, Python and C are actually similar: the true
default depends on code, not the signature.

> In many (most?) cases, code treats absent just like one specific present
> value.  We could lift this from code to the schema: permit specifying a
> default value in the schema, default absent to this value, don't
> generate has_FOO.
>

The problem is when the default is something dynamically determined;
we couldn't specify a static default. Maybe you regard that as a
feature! but it's probably a lot of work to turn back the hands of
time there now, so...

> The remainder could then be handled as the ugly wart it is: have both
> has_FOO and FOO in Python, just like in C.  Or handle it using one of
> the ideas you describe next.

There's no way to do that automatically that I know of... without
resorting to type signatures that utilize **kwargs, which are
untypable and don't reveal their arguments upon introspection. I think
it's just not possible.

>
> > So how do we write a function signature that conveys the difference
> > between "omitted" and "explicitly nulled" ...?
> >
> > One common trick in Python is to create a new sentinel singleton, and
> > name it something like "Default" or "Unspecified" or "Undefined". Many
> > programmers use the ellipsis `...` value for this purpose. Then, we
> > can check if a value was omitted (`...`) or explicitly provided
> > (`None`). It is very unlikely that these sentinels would organically
> > collide with user-provided values (Unless they were trying to
> > explicitly invoke default behavior.)
> >
> > However, `...` isn't supported as a type and using it as the default
> > value invalidates the typing of the field. As far as I can tell, it
> > CANNOT be typed. We could create our own sentinel, but IMO, this
> > creates a much less readable signature:
> >
> > async def example_command(arg: Union[int, qmp.Default] = qmp.Default)
> > -> None: ...
> >
> > This probably doesn't communicate "This parameter is actually
> > optional" to a casual Python programmer, so I think it's a dead end.
> >
> > The last thing I can think of here is to instead introduce a special
> > sentinel that represents the explicit Null instead. We could use a
> > special Null() type that means "Explicitly send a null over the wire."
> >
> > This value comes up fairly infrequently, so most signatures will
> > appear "Pythonic" and the jankiness will be confined to the few
> > commands that require it, e.g.
> >
> > async def example_command(arg: Optional[Union[int, Null]] = None) -> None: ...
> >
> > The above would imply an optional argument that can be omitted, can be
> > provided with an int, or can be provided with an explicit Null. I
> > think this is a good compromise.
>
> More so, I think, if we manage to substantially reduce "absent is
> distinct from any present value".
>

It only seems to show up a handful of times so far, it's not very
widespread as-is. Keeping it from spreading would be good, I assume.
Sweeping the dust into this particular corner seems like the correct
way to minimize API weirdness.

> > (5) Generating functions from introspection data is difficult because
> > all of the structures are anonymous.
>
> Introspection is about the wire format.  Type names are not part of it,
> and that's why we hide them in introspection.
>

Yep. I'm not asking for that to change, necessarily. Just saying "Hey,
I tried to autogenerate an SDK based on Introspection data and YOU
WON'T BELIEVE WHAT HAPPENED NEXT".

Except you'd believe it, because you designed it. For me, trying to do
it and seeing the result was the fastest way to truly come to terms
with how introspection data worked. Some of the lessons learned from
this exercise also apply to the task of generating function signatures
in general, too.

> >                                      The base type for most objects
> > becomes `Dict[str, Any]` but this isn't very descriptive. For Python
> > 3.8+, we can do a little better and use `Dict[Literal["name", "node"],
> > Any]` to help suggest what keys are valid, but we don't have access to
> > an in-line definition that pairs key names with values.
> >
> > Python 3.8+ would allow us the use of TypedDict, but those have to be
> > generated separately ... AND we still don't have a name for them, so
> > it'd be a little hogwash to have a function like:
> >
> > async def some_command(arg: Anon321) -> None: ...
> >
> > That doesn't really tell me, the human, much of anything. The best
> > that could perhaps be done is to create type aliases based on the name
> > of the argument it is the data type for, like "ArgObject". It's a bit
> > messy. For now, I've just stuck with the boring `Dict[Literal[...],
> > Any]` definition.
>
> I'm afraid you're trying to press introspection into a role it's not
> designed for.

Yes.

>
> Why not generate from the QAPI schema?

I was hoping you'd say that! =)
GOOD NEWS: I have also done that!

I tried it both ways to find the strengths and weaknesses of each
approach so I would have an informed basis for upstreaming the code
for either. The dynamic runtime method has its uses in that it will
always match the host API exactly. It cannot be used for static type
analysis of a script, though (The code doesn't exist until you connect
to the server!). The statically generated version has a lot more
information to work with and can be used for static type analysis, but
includes some parameters and functions that may not exist on your
server's instance.

The two approaches aren't even necessarily mutually exclusive.

>
> > (6) Dealing with variants is hard. I didn't get a working
>
> Do you mean alternates?
>

No, alternates at the python signature level are easy: Union[BranchA, BranchB].

In my code, I stubbed out the case where 'meta-type': 'object' has a
'variants' key. There's no blocker here, I just didn't get around to
un-stubbing it. I need to refactor a bit to make this work -- this
requires some flattening in the client code, unlike "normal" objects
which come pre-flattened over the wire. Just remarking that it appears
to be the most complex case for digesting introspection information.

> > implementation for them within one day of hacking, so I stubbed them
> > out. There's no major blocker here, just reporting that I still have
> > to finish this part of the experiment. I'm pretty happy that Markus
> > simplified the union types we have, though. To my knowledge, I got
> > everything else working perfectly.
> >
> > (7) I have no idea what to do about functions that "may not return".
> > The QGA stuff in particular, I believe, is prone to some weirdness
> > that violates the core principles of the QMP spec.
>
> Yes.
>
> docs/interop/qmp-spec.txt dictates a command sends either a success or
> an error response.  Makes sense.
>
> QGA has a few commands that shut down the guest.  How could such a
> command send a success response?  If it sends it before it initiates
> shutdown, response transmission races with shutdown.  The easy way out
> is violating qmp-spec.txt.  Thus, 'success-response': false.  Just for
> QGA.
>

Oh, whoops, I already have the information we need. O:-)
(Assuming that 'success-response' is visible in the introspection data, anyway.)


> >                                                    Maybe we can add a
> > "NORETURN" feature flag to those commands in the schema so that
> > clients can be aware of which commands may break the expectation of
> > always getting an RPC reply?
>
> For a normal command, bindings marshal, send the command, receive the
> response, unmarshal.
>
> For a 'success-response': false command, they only marshal and send.
>
> > (8) Thanks for reading. I'm still buried under my holiday inbox, but I
> > am trying like hell to catch up on everything. I know I missed a few
> > calls in which API design was discussed, and I apologize for that.
> > Please send me invitations using "to: jsnow@redhat.com" to ensure I do
> > not miss them. I am also frantically trying to clean up the Async QMP
> > project I was working on to have more mental bandwidth for other
> > tasks, but it's dragging on a bit longer than I had anticipated.
> > Please accept my apologies for being somewhat reclusive lately.
> >
> > I'll (try to) send a status overview of the various projects I'm
> > working on later to help set priority and discuss with the community
> > what my goals are and what I'd like to do. I have an awful lot of code
> > I've built up in local branches that I would like to share, but I'm
> > already sending code upstream as fast as I can, so maybe I'll just do
> > an overview at some point and point to unfinished code/experiments so
> > it's at least not completely unwitnessed work.
> >
> > I hope 2022 is treating you all well,
>
> Happy new year to you, too!
>

How is it already February?

Thanks for taking a look at my notes!
--js

Re: Notes on Generating Python signatures for QMP RPCs

[Markus Armbruster]

John Snow <jsnow@redhat.com> writes:

> On Thu, Jan 27, 2022 at 9:03 AM Markus Armbruster <armbru@redhat.com> wrote:
>>
>> John Snow <jsnow@redhat.com> writes:
>>
>> > Hiya, I was experimenting with $subject and ran into a few points of
>> > interest. This is basically an informal status report from me. I've
>> > CC'd some of the usual suspects for people who care about SDKs and API
>> > design and such.
>> >
>> > This is just a list of some observations I had, so not everything
>> > below is a question or an action item. Just sharing some notes.
>> >
>> > (0) This experiment concerned generating signatures based on
>> > introspection data, dynamically at runtime. In this environment type
>> > hints are not required, as they are not actually used at runtime.
>> > However, I added them anyway as an exercise for dynamic documentation
>> > purposes. (i.e. `help(auto_generated_function)` showing type hints can
>> > still be useful -- especially without access to QAPI doc blocks.)
>> > Determining type information is also necessary for generating the
>> > marshaling/unmarshaling functions to communicate with the server.
>> >
>> > (1) QAPI types the return of many commands as an empty object. That's
>> > literally indeed what happens on the wire, and it makes sense in that
>> > if these commands were ever to return anything, it is a "compatible
>> > evolution" to include new fields in such an object. In Python, this
>> > does not make much sense, though; as this is somewhat hard to
>> > annotate:
>> >
>> > async def stop() -> Literal[{}]: ...
>> >
>> > The more pythonic signature is:
>> >
>> > async def stop() -> None: ...
>> >
>> > I feel like it's spiritually equivalent, but I am aware it is a
>> > distinct choice that is being made. It could theoretically interfere
>> > with a choice made in QAPI later to explicitly return Null. I don't
>> > think we'd do that, but it's still a choice of abstraction that
>> > reduces the resolution of distinct return signatures.
>>
>> Having functions take any number of arguments and return one result is
>> traditional in programming, so we hardly notice how odd the asymmetry
>> actually is.
>>
>> QAPI commands are symmetric: they take any number of (named) arguments,
>> and return any number of (named) results.  Can also be viewed as taking
>> one exactly one argument (always an object) and returning exactly one
>> value (always an object).
>>
>> Except some QAPI commands actually return return a list of objects, and
>> some return a single, unnamed value.  The former are intentional, the
>> latter are screwups from the early days.
>>
>> > (1.5) Do we have a formal definition for what we consider to be a
>> > "compatible evolution" of the schema?
>>
>> Section "Compatibility considerations" in docs/devel/qapi-code-gen.rst
>> tries.  I wouldn't call it "a formal definition".
>>
>> >                                       I've got a fairly good idea, but
>> > I am not sure how it's enforced. Is it just Markus being very
>> > thorough?
>>
>> Yup, with help from other reviewers such as Eric Blake, and enabled by
>> the relative simplicity of the rules.
>>
>> >           If we add more languages to the generator, we probably can't
>> > burden Markus with knowing how to protect the compatibility of every
>> > generator.
>>
>> qapi-code-gen.rst applies the compatibility razor to the wire protocol
>> only.  It doesn't concern itself with language bindings at all.
>>
>
> The generator code certainly does, though.

Correct.  Since the generator generates C bindings, it must care about
language bindings (one).

What it doesn't concern itself so far with is *stability* of this
binding: it's just for use within QEMU.

>                                            For example, "C name"
> collisions are guarded against by the generator -- we do not let it
> get as far as blowing up clang.

"Guards against name collisions" feels like an overly charitable
description.  "Makes an effort" is closer to the truth.

>> We've only ever had C bindings.  Designed for internal use only, where
>> compatible evolution is pretty much irrelevant.
>>
>> We've talked about bindings for external use, both long ago (C), and
>> more recently (Rust, Python, ...).
>>
>> [*] Compatible evolution of language bindings for QAPI is a complex
>> topic.  Not today.
>>
>
> OK, agreed. It's a big topic. I think it's not fully possible to cover
> all cases in all places, but I do seem to recall Marc-Andre having
> some opinions about a few things we could do to staple it down just a
> little bit, and maybe that's enough. I'd like to talk about it
> eventually. Marc-Andre could cover rust, I could cover Python, and
> Victor/Andrea could help us cover golang.

Yes, it's something we should talk about at some point.

>> >            We might need more assertions for invariants in the
>> > generator itself ... but to protect "evolution", we need points of
>> > reference to test against. Do we have anything for this? Do we need
>> > one? Should I write a test?
>>
>> So far, we don't have software to reason about QAPI schema *changes*.
>> The generator checks the current QAPI schema, it doesn't consider the
>> change from the previous version.
>>
>> A program to analyze a schema change (the difference between two schema
>> versions) could be useful.  It could also be difficult to write.
>>
>
> Yeh. Something not worth doing until we chat about what compatible
> evolution looks like for other language bindings, because otherwise we
> don't know what we're designing for. Later.
>
>> > (2) There are five commands that are exempted from returning an
>> > object. qom-get is one.
>>
>> This is pragma 'command-returns-exceptions'.  Like its buddy pragmas, it
>> exists because we started enforcing the rules only after our rule
>> breaking had calcified in the external interface.
>>
>> >                         However, what I didn't really explicitly
>> > realize is that this doesn't mean that only five commands don't return
>> > an object -- we also actually allow for a list of objects, which
>> > *many* commands use. There's no technical issue here, just an
>> > observation. It is no problem at all to annotate Python commands as
>> > "-> SomeReturnType" or "-> List[SomeDifferentReturnType]" or even "->
>> > str:" as needed.
>> >
>> > (3) Over the wire, the order of arguments to QMP commands is not
>> > specified.
>>
>> Correct.
>>
>> >            In generating commands procedurally from introspection
>> > data, I am made aware that there are several commands in which
>> > "optional" arguments precede "required" arguments. This means that
>> > function generation in Python cannot match the stated order 1:1.
>> >
>> > That's not a crisis per se. For generating functions, we can use a
>> > stable sort to bubble-up the required arguments, leaving the optional
>> > ones trailing. However, it does mean that depending on how the QAPI
>> > schema is modified in the future, the argument order may change
>> > between versions of a generative SDK. I'd like to avoid that, if I
>> > can.
>> >
>> > One trick I have available to me in Python is the ability to stipulate
>> > that all (QAPI) "optional" arguments are keyword-only. This means that
>> > Optional parameters can be re-ordered arbitrarily without any breakage
>> > in the generative python API. The only remaining concern is if the
>> > *mandatory* arguments are re-ordered.
>> >
>> > (In fact, I could stipulate that ALL arguments in Python bindings are
>> > keyword-only, but I think that's going overboard and hurts usability
>> > and readability.)
>>
>> This would match the wire protocol, which only uses keyword, never
>> positional.  But your usability argument is valid.
>>
>> > Marc-Andre has mentioned this before, but it might be nice to actually
>> > specify a canonical ordering of arguments for languages that require
>> > such things, and to make sure that we do not break this ordering
>> > without good reason.
>> >
>> > (Of course, SDK stability is not fully possible, and if this
>> > functionality is desired, then it's time to use libvirt, hint hint
>> > hint! However, we can avoid pointless churn in generated code and make
>> > it easier to use and experiment with.)
>>
>> See [*] above.
>>
>> > (4) StrOrNull is a tricky design problem.
>> >
>> > In Python, generally, omitted arguments are typed like this:
>> > async def example_command(arg: Optional[int] = None) -> None: ...
>> >
>> > Most Python programmers would recognize that signature as meaning that
>> > they can omit 'arg' and some hopefully good default will be chosen.
>> > However, in QAPI we do have the case where "omitted" is distinct from
>> > "explicitly provided null". This is ... a bit challenging to convey
>> > semantically. Python does not offer the ability to tell "what kind of
>> > None" it received; i.e. unlike our generated QMP marshalling
>> > functions, we do not have a "has_arg" boolean we can inspect.
>>
>> Yes.
>>
>> The QAPI schema language's semantics of optional are at odds with
>> Python's.
>>
>> In Python, an absent argument defaults to some value, either one the
>> programmer provided, or None.  This makes semantics of "absent" obvious
>> from the signature.
>>
>
> "None" is never an assumed default in Python, it's always explicitly
> assigned as part of the signature. It is just by principle of least
> surprise that we all agree to use "None" as that default.

You're right.

>> In QAPI, an absent value is distinct from any present value.  Semantics
>> of "absent" depend the function's *code*, not just its signature.  I
>> consider this a design mistake.
>>
>
> Because of the above, Python and C are actually similar: the true
> default depends on code, not the signature.

Do you mean QAPI and C?

>> In many (most?) cases, code treats absent just like one specific present
>> value.  We could lift this from code to the schema: permit specifying a
>> default value in the schema, default absent to this value, don't
>> generate has_FOO.
>>
>
> The problem is when the default is something dynamically determined;
> we couldn't specify a static default. Maybe you regard that as a
> feature! but it's probably a lot of work to turn back the hands of
> time there now, so...

I suspect dynamic defaults are fairly rare.

Even when you "only" have Python's "absent defaults to a value, and you
can't distinguish absent from present with that value", you can still
code up dynamic defaults.  It takes a bit more effort than a static
default, but I *like* that.  When making things more complex takes more
effort, the chances of complex getting chosen only when it's genuinely
useful improve :)

>> The remainder could then be handled as the ugly wart it is: have both
>> has_FOO and FOO in Python, just like in C.  Or handle it using one of
>> the ideas you describe next.
>
> There's no way to do that automatically that I know of... without
> resorting to type signatures that utilize **kwargs, which are
> untypable and don't reveal their arguments upon introspection. I think
> it's just not possible.
>
>>
>> > So how do we write a function signature that conveys the difference
>> > between "omitted" and "explicitly nulled" ...?
>> >
>> > One common trick in Python is to create a new sentinel singleton, and
>> > name it something like "Default" or "Unspecified" or "Undefined". Many
>> > programmers use the ellipsis `...` value for this purpose. Then, we
>> > can check if a value was omitted (`...`) or explicitly provided
>> > (`None`). It is very unlikely that these sentinels would organically
>> > collide with user-provided values (Unless they were trying to
>> > explicitly invoke default behavior.)
>> >
>> > However, `...` isn't supported as a type and using it as the default
>> > value invalidates the typing of the field. As far as I can tell, it
>> > CANNOT be typed. We could create our own sentinel, but IMO, this
>> > creates a much less readable signature:
>> >
>> > async def example_command(arg: Union[int, qmp.Default] = qmp.Default)
>> > -> None: ...
>> >
>> > This probably doesn't communicate "This parameter is actually
>> > optional" to a casual Python programmer, so I think it's a dead end.
>> >
>> > The last thing I can think of here is to instead introduce a special
>> > sentinel that represents the explicit Null instead. We could use a
>> > special Null() type that means "Explicitly send a null over the wire."
>> >
>> > This value comes up fairly infrequently, so most signatures will
>> > appear "Pythonic" and the jankiness will be confined to the few
>> > commands that require it, e.g.
>> >
>> > async def example_command(arg: Optional[Union[int, Null]] = None) -> None: ...
>> >
>> > The above would imply an optional argument that can be omitted, can be
>> > provided with an int, or can be provided with an explicit Null. I
>> > think this is a good compromise.
>>
>> More so, I think, if we manage to substantially reduce "absent is
>> distinct from any present value".
>>
>
> It only seems to show up a handful of times so far, it's not very
> widespread as-is. Keeping it from spreading would be good, I assume.
> Sweeping the dust into this particular corner seems like the correct
> way to minimize API weirdness.
>
>> > (5) Generating functions from introspection data is difficult because
>> > all of the structures are anonymous.
>>
>> Introspection is about the wire format.  Type names are not part of it,
>> and that's why we hide them in introspection.
>>
>
> Yep. I'm not asking for that to change, necessarily. Just saying "Hey,
> I tried to autogenerate an SDK based on Introspection data and YOU
> WON'T BELIEVE WHAT HAPPENED NEXT".
>
> Except you'd believe it, because you designed it. For me, trying to do
> it and seeing the result was the fastest way to truly come to terms
> with how introspection data worked. Some of the lessons learned from
> this exercise also apply to the task of generating function signatures
> in general, too.
>
>> >                                      The base type for most objects
>> > becomes `Dict[str, Any]` but this isn't very descriptive. For Python
>> > 3.8+, we can do a little better and use `Dict[Literal["name", "node"],
>> > Any]` to help suggest what keys are valid, but we don't have access to
>> > an in-line definition that pairs key names with values.
>> >
>> > Python 3.8+ would allow us the use of TypedDict, but those have to be
>> > generated separately ... AND we still don't have a name for them, so
>> > it'd be a little hogwash to have a function like:
>> >
>> > async def some_command(arg: Anon321) -> None: ...
>> >
>> > That doesn't really tell me, the human, much of anything. The best
>> > that could perhaps be done is to create type aliases based on the name
>> > of the argument it is the data type for, like "ArgObject". It's a bit
>> > messy. For now, I've just stuck with the boring `Dict[Literal[...],
>> > Any]` definition.
>>
>> I'm afraid you're trying to press introspection into a role it's not
>> designed for.
>
> Yes.
>
>>
>> Why not generate from the QAPI schema?
>
> I was hoping you'd say that! =)
> GOOD NEWS: I have also done that!

Well played, sir!

> I tried it both ways to find the strengths and weaknesses of each
> approach so I would have an informed basis for upstreaming the code
> for either. The dynamic runtime method has its uses in that it will
> always match the host API exactly. It cannot be used for static type
> analysis of a script, though (The code doesn't exist until you connect
> to the server!). The statically generated version has a lot more
> information to work with and can be used for static type analysis, but
> includes some parameters and functions that may not exist on your
> server's instance.
>
> The two approaches aren't even necessarily mutually exclusive.
>
>>
>> > (6) Dealing with variants is hard. I didn't get a working
>>
>> Do you mean alternates?
>>
>
> No, alternates at the python signature level are easy: Union[BranchA, BranchB].
>
> In my code, I stubbed out the case where 'meta-type': 'object' has a
> 'variants' key. There's no blocker here, I just didn't get around to
> un-stubbing it. I need to refactor a bit to make this work -- this
> requires some flattening in the client code, unlike "normal" objects
> which come pre-flattened over the wire. Just remarking that it appears
> to be the most complex case for digesting introspection information.
>
>> > implementation for them within one day of hacking, so I stubbed them
>> > out. There's no major blocker here, just reporting that I still have
>> > to finish this part of the experiment. I'm pretty happy that Markus
>> > simplified the union types we have, though. To my knowledge, I got
>> > everything else working perfectly.
>> >
>> > (7) I have no idea what to do about functions that "may not return".
>> > The QGA stuff in particular, I believe, is prone to some weirdness
>> > that violates the core principles of the QMP spec.
>>
>> Yes.
>>
>> docs/interop/qmp-spec.txt dictates a command sends either a success or
>> an error response.  Makes sense.
>>
>> QGA has a few commands that shut down the guest.  How could such a
>> command send a success response?  If it sends it before it initiates
>> shutdown, response transmission races with shutdown.  The easy way out
>> is violating qmp-spec.txt.  Thus, 'success-response': false.  Just for
>> QGA.
>>
>
> Oh, whoops, I already have the information we need. O:-)
> (Assuming that 'success-response' is visible in the introspection data, anyway.

qapi/introspect.json:

    ##
    # @SchemaInfoCommand:
    [...]
    # TODO: @success-response (currently irrelevant, because it's QGA, not QMP)
    #
    # Since: 2.5
    ##
    { 'struct': 'SchemaInfoCommand',
      'data': { 'arg-type': 'str', 'ret-type': 'str',
                '*allow-oob': 'bool' } }

The TODO neglects to spell out "and QGA doesn't support introspection so
far".

>> >                                                    Maybe we can add a
>> > "NORETURN" feature flag to those commands in the schema so that
>> > clients can be aware of which commands may break the expectation of
>> > always getting an RPC reply?
>>
>> For a normal command, bindings marshal, send the command, receive the
>> response, unmarshal.
>>
>> For a 'success-response': false command, they only marshal and send.
>>
>> > (8) Thanks for reading. I'm still buried under my holiday inbox, but I
>> > am trying like hell to catch up on everything. I know I missed a few
>> > calls in which API design was discussed, and I apologize for that.
>> > Please send me invitations using "to: jsnow@redhat.com" to ensure I do
>> > not miss them. I am also frantically trying to clean up the Async QMP
>> > project I was working on to have more mental bandwidth for other
>> > tasks, but it's dragging on a bit longer than I had anticipated.
>> > Please accept my apologies for being somewhat reclusive lately.
>> >
>> > I'll (try to) send a status overview of the various projects I'm
>> > working on later to help set priority and discuss with the community
>> > what my goals are and what I'd like to do. I have an awful lot of code
>> > I've built up in local branches that I would like to share, but I'm
>> > already sending code upstream as fast as I can, so maybe I'll just do
>> > an overview at some point and point to unfinished code/experiments so
>> > it's at least not completely unwitnessed work.
>> >
>> > I hope 2022 is treating you all well,
>>
>> Happy new year to you, too!
>>
>
> How is it already February?

Crazy, isn't it?

> Thanks for taking a look at my notes!

You're welcome!

Re: Notes on Generating Python signatures for QMP RPCs

[Daniel P. Berrangé]

On Wed, Jan 26, 2022 at 01:58:19PM -0500, John Snow wrote:
> (1) QAPI types the return of many commands as an empty object. That's
> literally indeed what happens on the wire, and it makes sense in that
> if these commands were ever to return anything, it is a "compatible
> evolution" to include new fields in such an object. In Python, this
> does not make much sense, though; as this is somewhat hard to
> annotate:
> 
> async def stop() -> Literal[{}]: ...
> 
> The more pythonic signature is:
> 
> async def stop() -> None: ...
> 
> I feel like it's spiritually equivalent, but I am aware it is a
> distinct choice that is being made. It could theoretically interfere
> with a choice made in QAPI later to explicitly return Null. I don't
> think we'd do that, but it's still a choice of abstraction that
> reduces the resolution of distinct return signatures.

As you mention though, bear in mind that a command returning
nothing today, might return something tomorrow. IOW, we'd be
going from a empty dict to a non-empty dict. If you use "None"
then it'd be gonig from None to a non-empty dict.

I think you can argue both of these approaches are backwards
compatible. The python app is not likely to be looking at
the return type at all initially - unlike C, errors get
raised as exceptions, so you don't need to look at return
type to distinguish succes from failure. 

So I'd consider it merely a documentation problem to say
that a "None" return type might later change to a dict.
Dunno how you represent that in python type annotations,
but I presume there's a way.

> (1.5) Do we have a formal definition for what we consider to be a
> "compatible evolution" of the schema? I've got a fairly good idea, but
> I am not sure how it's enforced. Is it just Markus being very
> thorough? If we add more languages to the generator, we probably can't
> burden Markus with knowing how to protect the compatibility of every
> generator. We might need more assertions for invariants in the
> generator itself ... but to protect "evolution", we need points of
> reference to test against. Do we have anything for this? Do we need
> one? Should I write a test?

It isn't enforced through any technical measures. For example just
recently we accidentally broke back compat of query-sgx by deleting
a field.

From the POV of defining "compatible evolution" I guess I'd
ask what constraints you envisage being important from your
Python code generator POV ?

We do allow fields to be deleted, which is a *non-compatible*
evolution, but they MUST have been marked as deprecated for
2 cycles first.

Because the C generated code is only used inside QEMU, when
we have intentional non-compatible changes, we merely update
the callers in QEMU as needed.

If you are anticipating the python generated code to be a
publically consumable API this becomes a bit more complex
problem as you can't rely on fixing callers.

> (3) Over the wire, the order of arguments to QMP commands is not
> specified. In generating commands procedurally from introspection
> data, I am made aware that there are several commands in which
> "optional" arguments precede "required" arguments. This means that
> function generation in Python cannot match the stated order 1:1.
> 
> That's not a crisis per se. For generating functions, we can use a
> stable sort to bubble-up the required arguments, leaving the optional
> ones trailing. However, it does mean that depending on how the QAPI
> schema is modified in the future, the argument order may change
> between versions of a generative SDK. I'd like to avoid that, if I
> can.

I'd say sorting required vs optional arguments is doomed as
a strategy. Stuff that is mandatory today can be made optional
tomorrow and I think that's reasonable to want todo as we
evolve an API design.

Consider for example a command for querying something about
a CPU.  It might take a mandatory CPU ID number as its
input parameter today. It could then be changed to accept
either a CPU ID or a QOM Path, and both would be marked
optional but at least one would need to set.

This is backwards compatible from POV of callers because
anyone passing a CPU ID today can carry on passing a CPU
ID. New callers can choose to use QOM path instead.

So I think however you express API params in python needs
to cope with this scenario, which means not sorting
args based on optional vs required. Effectively need
to assume that all args are potentially optional on a
long enough timeframe.

> One trick I have available to me in Python is the ability to stipulate
> that all (QAPI) "optional" arguments are keyword-only. This means that
> Optional parameters can be re-ordered arbitrarily without any breakage
> in the generative python API. The only remaining concern is if the
> *mandatory* arguments are re-ordered.
> 
> (In fact, I could stipulate that ALL arguments in Python bindings are
> keyword-only, but I think that's going overboard and hurts usability
> and readability.)

I don't think you have any choice - they must all be keyword
only if you want protection from future schema changes.

> Marc-Andre has mentioned this before, but it might be nice to actually
> specify a canonical ordering of arguments for languages that require
> such things, and to make sure that we do not break this ordering
> without good reason.

Declaring a canonical ordering is not unreasonable as a concept
on the surface. Essentially this is akin to fixing the order of
fields in a C struct and making it append-only.

None the less if you rely on this for positional arguments in
python callers are still going to break when we *intentionally*
delete fields/parameters (after a deprecation cycle). WIth
strictly keyword only args, if the caller was not using the
deleted field they won't be affected by the deletion as they
won't be position sensitive.

> (4) StrOrNull is a tricky design problem.
> 
> In Python, generally, omitted arguments are typed like this:
> async def example_command(arg: Optional[int] = None) -> None: ...
> 
> Most Python programmers would recognize that signature as meaning that
> they can omit 'arg' and some hopefully good default will be chosen.
> However, in QAPI we do have the case where "omitted" is distinct from
> "explicitly provided null". This is ... a bit challenging to convey
> semantically. Python does not offer the ability to tell "what kind of
> None" it received; i.e. unlike our generated QMP marshalling
> functions, we do not have a "has_arg" boolean we can inspect.
> 
> So how do we write a function signature that conveys the difference
> between "omitted" and "explicitly nulled" ...?
> 
> One common trick in Python is to create a new sentinel singleton, and
> name it something like "Default" or "Unspecified" or "Undefined". Many
> programmers use the ellipsis `...` value for this purpose. Then, we
> can check if a value was omitted (`...`) or explicitly provided
> (`None`). It is very unlikely that these sentinels would organically
> collide with user-provided values (Unless they were trying to
> explicitly invoke default behavior.)
> 
> However, `...` isn't supported as a type and using it as the default
> value invalidates the typing of the field. As far as I can tell, it
> CANNOT be typed. We could create our own sentinel, but IMO, this
> creates a much less readable signature:
> 
> async def example_command(arg: Union[int, qmp.Default] = qmp.Default)
> -> None: ...
> 
> This probably doesn't communicate "This parameter is actually
> optional" to a casual Python programmer, so I think it's a dead end.

It sounds like you need a wrapper type.  This StrOrNull scenario
is QMP's "alternate" type IIUC, but you're trying to avoid
expressing the existance fo the "alternate" type in the API

ie you're trying to support

    example_command("foo") 
    example_command(None)
    example_command()

but that's impossible as the last 2 can't be distinguished

If you explicitly huave an "Alternate" object type, which
is a wrapper for any other type, then you can do

    example_command(Alternate("foo")) 
    example_command(Alternate(None))
    example_command()

and now be able to distinguish the different scenarios.

> (5) Generating functions from introspection data is difficult because
> all of the structures are anonymous. The base type for most objects
> becomes `Dict[str, Any]` but this isn't very descriptive. For Python
> 3.8+, we can do a little better and use `Dict[Literal["name", "node"],
> Any]` to help suggest what keys are valid, but we don't have access to
> an in-line definition that pairs key names with values.

Yep, we've also taken advantage of this to rename structs
periodically as while it affected the generated C code
inside QEMU, it didn't affect any QMP clients.

I think it is not unreasonable to expose the struct names
on introspection though, and just accept that it ties our
hands a little more to avoid renaming structs. I don't
think we rename frequently enough that this matters.

> (6) Dealing with variants is hard. I didn't get a working
> implementation for them within one day of hacking, so I stubbed them
> out. There's no major blocker here, just reporting that I still have
> to finish this part of the experiment. I'm pretty happy that Markus
> simplified the union types we have, though. To my knowledge, I got
> everything else working perfectly.

Variants feels like it is the same kind of problem space
as the StrOrNone scenario earlier.

> (7) I have no idea what to do about functions that "may not return".
> The QGA stuff in particular, I believe, is prone to some weirdness
> that violates the core principles of the QMP spec. Maybe we can add a
> "NORETURN" feature flag to those commands in the schema so that
> clients can be aware of which commands may break the expectation of
> always getting an RPC reply?

A "NORETURN" flag sounds like a reasonable idea.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: Notes on Generating Python signatures for QMP RPCs

[John Snow]

On Thu, Feb 3, 2022 at 5:04 AM Markus Armbruster <armbru@redhat.com> wrote:
> John Snow <jsnow@redhat.com> writes:
> > On Thu, Jan 27, 2022 at 9:03 AM Markus Armbruster <armbru@redhat.com> wrote:
> >> John Snow <jsnow@redhat.com> writes:

> >> > (7) I have no idea what to do about functions that "may not return".
> >> > The QGA stuff in particular, I believe, is prone to some weirdness
> >> > that violates the core principles of the QMP spec.
> >>
> >> Yes.
> >>
> >> docs/interop/qmp-spec.txt dictates a command sends either a success or
> >> an error response.  Makes sense.
> >>
> >> QGA has a few commands that shut down the guest.  How could such a
> >> command send a success response?  If it sends it before it initiates
> >> shutdown, response transmission races with shutdown.  The easy way out
> >> is violating qmp-spec.txt.  Thus, 'success-response': false.  Just for
> >> QGA.
> >>
> >
> > Oh, whoops, I already have the information we need. O:-)
> > (Assuming that 'success-response' is visible in the introspection data, anyway.
>
> qapi/introspect.json:
>
>     ##
>     # @SchemaInfoCommand:
>     [...]
>     # TODO: @success-response (currently irrelevant, because it's QGA, not QMP)
>     #
>     # Since: 2.5
>     ##
>     { 'struct': 'SchemaInfoCommand',
>       'data': { 'arg-type': 'str', 'ret-type': 'str',
>                 '*allow-oob': 'bool' } }
>
> The TODO neglects to spell out "and QGA doesn't support introspection so
> far".
>

Oof, ouch, my bones.

What will it take to add introspection to QGA? (Is this GSoC/Outreachy
appropriate?)
(This is not critically important to me, just a backburner thought.)

--js

Re: Notes on Generating Python signatures for QMP RPCs

[John Snow]

On Thu, Feb 3, 2022 at 5:40 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Wed, Jan 26, 2022 at 01:58:19PM -0500, John Snow wrote:
> > (1) QAPI types the return of many commands as an empty object. That's
> > literally indeed what happens on the wire, and it makes sense in that
> > if these commands were ever to return anything, it is a "compatible
> > evolution" to include new fields in such an object. In Python, this
> > does not make much sense, though; as this is somewhat hard to
> > annotate:
> >
> > async def stop() -> Literal[{}]: ...
> >
> > The more pythonic signature is:
> >
> > async def stop() -> None: ...
> >
> > I feel like it's spiritually equivalent, but I am aware it is a
> > distinct choice that is being made. It could theoretically interfere
> > with a choice made in QAPI later to explicitly return Null. I don't
> > think we'd do that, but it's still a choice of abstraction that
> > reduces the resolution of distinct return signatures.
>
> As you mention though, bear in mind that a command returning
> nothing today, might return something tomorrow. IOW, we'd be
> going from a empty dict to a non-empty dict. If you use "None"
> then it'd be gonig from None to a non-empty dict.
>
> I think you can argue both of these approaches are backwards
> compatible. The python app is not likely to be looking at
> the return type at all initially - unlike C, errors get
> raised as exceptions, so you don't need to look at return
> type to distinguish succes from failure.
>
> So I'd consider it merely a documentation problem to say
> that a "None" return type might later change to a dict.
> Dunno how you represent that in python type annotations,
> but I presume there's a way.

I don't think type hints offer a temporal dimension to them yet :)

I started writing a much lengthier response, but the subject of
compatibility is really complex and I am not prepared to give a
comprehensive response to some of the issues you raise ... so instead
I will say "Wow, good points!" and I will get back to you on some of
it. A lot of things will only make sense if we are talking about a
very specific type of project, with very specific goals that are
clearly established. I don't really have that ready, here; I am just
experimenting to learn where some of the pain points are, still.

So... I'll get back to you on this.

> We do allow fields to be deleted, which is a *non-compatible*
> evolution, but they MUST have been marked as deprecated for
> 2 cycles first.

Good point.

> I'd say sorting required vs optional arguments is doomed as
> a strategy. Stuff that is mandatory today can be made optional
> tomorrow and I think that's reasonable to want todo as we
> evolve an API design.

Also a good point. Python requires all mandatory arguments precede all
optional ones, so you're probably right that in order to maximize
cross-version compatibility, keyword-only arguments for *all*
arguments both mandatory and optional is the only real way to fly.

I think this might cause problems for Marc-Andre in rust/dbus land,
but it's tractable in Python. I am unclear on the ramifications for
golang. (Victor, Marc-Andre, any input here?)

[...]

> So I think however you express API params in python needs
> to cope with this scenario, which means not sorting
> args based on optional vs required. Effectively need
> to assume that all args are potentially optional on a
> long enough timeframe.

We still have to sort them to fulfill Python grammar requirements, but
if they are *all* keyword-only, then the order the programmer uses to
actually invoke the function isn't important.

> I don't think you have any choice - they must all be keyword
> only if you want protection from future schema changes.

You're right, it's simply more robust.

> It sounds like you need a wrapper type.  This StrOrNull scenario
> is QMP's "alternate" type IIUC, but you're trying to avoid
> expressing the existance fo the "alternate" type in the API

Yes. This is a very clean way to type it, but it is a little more
laborious for the user to have to remember to wrap certain strings in
a special constructor first. Still, this is a good trick that I hadn't
considered. I'll keep it in mind for future experiments.

> I think it is not unreasonable to expose the struct names
> on introspection though, and just accept that it ties our
> hands a little more to avoid renaming structs. I don't
> think we rename frequently enough that this matters.

I feel like I don't have a real stake in this issue yet. Maybe we can
discuss bolstering the introspection data if we decide that we really,
really would like the ability to generate bindings dynamically on the
client side. I'm not sure *I* even want that enough to really push for
this change yet. (Again, I think I need to come forward with something
more concrete than an experiment before I dive too deeply into this
issue. I'll get back to you.)

I wouldn't mind hearing from Markus on what he believes the value of
anonymizing the types names is. My current understanding is: "The type
names aren't necessary to speak QMP, so they aren't necessary to
reveal. We operate on a need-to-know basis, and nobody has needed to
know."

(The most tangible story I can think of is that we don't want clients
to do things like assume they can index the introspection data in a
hashmap and rely on looking up specific type names.)

> A "NORETURN" flag sounds like a reasonable idea.

Markus has gently pointed out that we do have this information in the
schema, but it isn't revealed over introspection data *and* we do not
have introspection for QGA anyway.

We /could/ remove success-response and add a 'NORETURN' feature flag,
modifying the generator to use that flag (instead of
success-response), and then we'd get away with not having to modify
the introspection schema. But we'd still have to add introspection in
general to QGA, which rather sounds like where the bulk of the work is
anyway.

>
> Regards,
> Daniel

Thanks! I've got a lot to think about.

--js

Re: Notes on Generating Python signatures for QMP RPCs

[Markus Armbruster]

John Snow <jsnow@redhat.com> writes:

> On Thu, Feb 3, 2022 at 5:04 AM Markus Armbruster <armbru@redhat.com> wrote:
>> John Snow <jsnow@redhat.com> writes:
>> > On Thu, Jan 27, 2022 at 9:03 AM Markus Armbruster <armbru@redhat.com> wrote:
>> >> John Snow <jsnow@redhat.com> writes:
>
>> >> > (7) I have no idea what to do about functions that "may not return".
>> >> > The QGA stuff in particular, I believe, is prone to some weirdness
>> >> > that violates the core principles of the QMP spec.
>> >>
>> >> Yes.
>> >>
>> >> docs/interop/qmp-spec.txt dictates a command sends either a success or
>> >> an error response.  Makes sense.
>> >>
>> >> QGA has a few commands that shut down the guest.  How could such a
>> >> command send a success response?  If it sends it before it initiates
>> >> shutdown, response transmission races with shutdown.  The easy way out
>> >> is violating qmp-spec.txt.  Thus, 'success-response': false.  Just for
>> >> QGA.
>> >>
>> >
>> > Oh, whoops, I already have the information we need. O:-)
>> > (Assuming that 'success-response' is visible in the introspection data, anyway.
>>
>> qapi/introspect.json:
>>
>>     ##
>>     # @SchemaInfoCommand:
>>     [...]
>>     # TODO: @success-response (currently irrelevant, because it's QGA, not QMP)
>>     #
>>     # Since: 2.5
>>     ##
>>     { 'struct': 'SchemaInfoCommand',
>>       'data': { 'arg-type': 'str', 'ret-type': 'str',
>>                 '*allow-oob': 'bool' } }
>>
>> The TODO neglects to spell out "and QGA doesn't support introspection so
>> far".
>
> Oof, ouch, my bones.
>
> What will it take to add introspection to QGA? (Is this GSoC/Outreachy
> appropriate?)
> (This is not critically important to me, just a backburner thought.)

The QEMU/QGA part should be easy enough: implement and document a
suitable introspection command, by stealing from query-qmp-schema.

The much more interesting part is putting it to actual use.

Re: Notes on Generating Python signatures for QMP RPCs

[Markus Armbruster]

John Snow <jsnow@redhat.com> writes:

> On Thu, Feb 3, 2022 at 5:40 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
>>
>> On Wed, Jan 26, 2022 at 01:58:19PM -0500, John Snow wrote:

[...]

>> I think it is not unreasonable to expose the struct names
>> on introspection though, and just accept that it ties our
>> hands a little more to avoid renaming structs. I don't
>> think we rename frequently enough that this matters.
>
> I feel like I don't have a real stake in this issue yet. Maybe we can
> discuss bolstering the introspection data if we decide that we really,
> really would like the ability to generate bindings dynamically on the
> client side. I'm not sure *I* even want that enough to really push for
> this change yet. (Again, I think I need to come forward with something
> more concrete than an experiment before I dive too deeply into this
> issue. I'll get back to you.)
>
> I wouldn't mind hearing from Markus on what he believes the value of
> anonymizing the types names is. My current understanding is: "The type
> names aren't necessary to speak QMP, so they aren't necessary to
> reveal. We operate on a need-to-know basis, and nobody has needed to
> know."
>
> (The most tangible story I can think of is that we don't want clients
> to do things like assume they can index the introspection data in a
> hashmap and rely on looking up specific type names.)

QMP's compatibility promise (which predates schema introspection)
applies to commands and events.

When I designed schema introspection, I didn't want to grow the existing
compatibility promise, because that would restrict what we can do in the
schema.  Instead, I opted to limit the new compatibility promise for
introspection.  Section "Client JSON Protocol introspection" in
docs/devel/qapi-code-gen.rst has a paragraph on it.

Since telling users what not to do has a somewhat disappointing success
rate, I additionally looked for easy ways to make things not to be done
impractical.  I found "hide the type names".  Tiny bonus: saves a bit of
bandwidth, which probably doesn't matter beyond pleasing me.

Deterring users from bad practice was arguably more important when
schema introspection was new, and good practice wasn't established.

commit 1a9a507b2e3e90aa719c96b4c092e7fad7215f21 (tag: pull-qapi-2015-09-21)
Author: Markus Armbruster <armbru@redhat.com>
Date:   Wed Sep 16 13:06:29 2015 +0200

    qapi-introspect: Hide type names
    
    To eliminate the temptation for clients to look up types by name
    (which are not ABI), replace all type names by meaningless strings.
    
    Reduces output of query-schema by 13 out of 85KiB.
    
    As a debugging aid, provide option -u to suppress the hiding.
    
    Signed-off-by: Markus Armbruster <armbru@redhat.com>
    Reviewed-by: Eric Blake <eblake@redhat.com>
    Message-Id: <1442401589-24189-27-git-send-email-armbru@redhat.com>

>> A "NORETURN" flag sounds like a reasonable idea.
>
> Markus has gently pointed out that we do have this information in the
> schema, but it isn't revealed over introspection data *and* we do not
> have introspection for QGA anyway.
>
> We /could/ remove success-response and add a 'NORETURN' feature flag,
> modifying the generator to use that flag (instead of
> success-response), and then we'd get away with not having to modify
> the introspection schema. But we'd still have to add introspection in
> general to QGA, which rather sounds like where the bulk of the work is
> anyway.

If we had feature flags from the start, we might not have added other
flags to the syntax, such as @gen, @success-response, @boxed.

Feature flags are exposed in introspection, the others aren't.  That
dictates which kind to use when adding a flag.

Flags that aren't exposed in introspection can only be used by the
generator itself (d'oh).

A few special feature flags are also used by the generator.  Currently
'deprecated' and 'unstable'.

>>
>> Regards,
>> Daniel
>
> Thanks! I've got a lot to think about.

I might have pile on some more, forgive me ;)

Re: Notes on Generating Python signatures for QMP RPCs

[Daniel P. Berrangé]

On Thu, Feb 03, 2022 at 05:52:10PM -0500, John Snow wrote:
> On Thu, Feb 3, 2022 at 5:40 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > On Wed, Jan 26, 2022 at 01:58:19PM -0500, John Snow wrote:
> >
> > As you mention though, bear in mind that a command returning
> > nothing today, might return something tomorrow. IOW, we'd be
> > going from a empty dict to a non-empty dict. If you use "None"
> > then it'd be gonig from None to a non-empty dict.
> >
> > I think you can argue both of these approaches are backwards
> > compatible. The python app is not likely to be looking at
> > the return type at all initially - unlike C, errors get
> > raised as exceptions, so you don't need to look at return
> > type to distinguish succes from failure.
> >
> > So I'd consider it merely a documentation problem to say
> > that a "None" return type might later change to a dict.
> > Dunno how you represent that in python type annotations,
> > but I presume there's a way.
> 
> I don't think type hints offer a temporal dimension to them yet :)
> 
> I started writing a much lengthier response, but the subject of
> compatibility is really complex and I am not prepared to give a
> comprehensive response to some of the issues you raise ... so instead
> I will say "Wow, good points!" and I will get back to you on some of
> it. A lot of things will only make sense if we are talking about a
> very specific type of project, with very specific goals that are
> clearly established. I don't really have that ready, here; I am just
> experimenting to learn where some of the pain points are, still.
> 
> So... I'll get back to you on this.
> 
> > We do allow fields to be deleted, which is a *non-compatible*
> > evolution, but they MUST have been marked as deprecated for
> > 2 cycles first.
> 
> Good point.
> 
> > I'd say sorting required vs optional arguments is doomed as
> > a strategy. Stuff that is mandatory today can be made optional
> > tomorrow and I think that's reasonable to want todo as we
> > evolve an API design.
> 
> Also a good point. Python requires all mandatory arguments precede all
> optional ones, so you're probably right that in order to maximize
> cross-version compatibility, keyword-only arguments for *all*
> arguments both mandatory and optional is the only real way to fly.
> 
> I think this might cause problems for Marc-Andre in rust/dbus land,
> but it's tractable in Python. I am unclear on the ramifications for
> golang. (Victor, Marc-Andre, any input here?)

Well as a general point, if we consider usage outside of
qemu.git, I'm far from convinced that generating code from
the schema as it exists today is going to be broadly usable
enough to make it worthwhile.

The problem is precisely that the code that is generated is
only ensured to work with the specific version of QEMU that
it was generated from. The key problem here is the removal
of features after deprecation.  That removal is fine if you
only ever need an API to talk to the current QEMU, or only
need to be able to introspect the current QEMU.

Management apps are likely to want to write code that works
with more than 1 version of QEMU, and be able to decide
whether they provide the params needed by the old command
or the new command.   The introspection data lets them
make the decision about which command needs to be invoked,
but it can't be used to generate the code needed for the
old command.

I don't know how exactly you're dealing with the Python
mapping. If you're literally statically generating Python
code you'll face this same problem. If on the other hand
you've just got a stub object that does dynamic dispatch
then it can dynamically adapt at runtime to work with
whatever version of the schema it queried from the QEMU
it is talking to. I'm hoping you're doing the latter
for this reason.

One strategy for compiled languages is to generate multiple
copies of the APIs, one for each QEMU version. This could
be made to work but I feel it is pretty horrific as an
approach.  Libvirt has one set of client APIs that we've
extended over time so they can be used to call both old
and new variants of commands - we just need to use the
introspected schema to decide which to use.

To make the latter viable for generating compiled code
though, we need to change how we deal with removals, by
essentially never removing things at all. Instead we
would need some way to express that "field A" or "type T"
is not present after some point in time / release.

Overall I don't think we're really in a position to use
compiled auto generated bindings, except for QMP clients
that are released in lockstep with specific QEMU versions,
and I don't think lockstep releases are a viable strategy
in general.

> > It sounds like you need a wrapper type.  This StrOrNull scenario
> > is QMP's "alternate" type IIUC, but you're trying to avoid
> > expressing the existance fo the "alternate" type in the API
> 
> Yes. This is a very clean way to type it, but it is a little more
> laborious for the user to have to remember to wrap certain strings in
> a special constructor first. Still, this is a good trick that I hadn't
> considered. I'll keep it in mind for future experiments.

Bear in mind that this situation is pretty rare, so I don't think
the user is especially burdened by needing an extra level of
indirection for using 'alternate' types

$ git grep "'alternate'" qapi
qapi/block-core.json:{ 'alternate': 'BlockDirtyBitmapMergeSource',
qapi/block-core.json:{ 'alternate': 'Qcow2OverlapChecks',
qapi/block-core.json:{ 'alternate': 'BlockdevRef',
qapi/block-core.json:{ 'alternate': 'BlockdevRefOrNull',
qapi/common.json:{ 'alternate': 'StrOrNull',

$ git grep "'StrOrNull'" qapi
qapi/block-core.json:             'iothread': 'StrOrNull',
qapi/common.json:{ 'alternate': 'StrOrNull',
qapi/migration.json:            '*tls-creds': 'StrOrNull',
qapi/migration.json:            '*tls-hostname': 'StrOrNull',
qapi/migration.json:            '*tls-authz': 'StrOrNull',


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: Notes on Generating Python signatures for QMP RPCs

[Markus Armbruster]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Thu, Feb 03, 2022 at 05:52:10PM -0500, John Snow wrote:
>> On Thu, Feb 3, 2022 at 5:40 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
>> >
>> > On Wed, Jan 26, 2022 at 01:58:19PM -0500, John Snow wrote:
>> >
>> > As you mention though, bear in mind that a command returning
>> > nothing today, might return something tomorrow. IOW, we'd be
>> > going from a empty dict to a non-empty dict. If you use "None"
>> > then it'd be gonig from None to a non-empty dict.
>> >
>> > I think you can argue both of these approaches are backwards
>> > compatible. The python app is not likely to be looking at
>> > the return type at all initially - unlike C, errors get
>> > raised as exceptions, so you don't need to look at return
>> > type to distinguish succes from failure.
>> >
>> > So I'd consider it merely a documentation problem to say
>> > that a "None" return type might later change to a dict.
>> > Dunno how you represent that in python type annotations,
>> > but I presume there's a way.
>> 
>> I don't think type hints offer a temporal dimension to them yet :)
>> 
>> I started writing a much lengthier response, but the subject of
>> compatibility is really complex and I am not prepared to give a
>> comprehensive response to some of the issues you raise ... so instead
>> I will say "Wow, good points!" and I will get back to you on some of
>> it. A lot of things will only make sense if we are talking about a
>> very specific type of project, with very specific goals that are
>> clearly established. I don't really have that ready, here; I am just
>> experimenting to learn where some of the pain points are, still.
>> 
>> So... I'll get back to you on this.
>> 
>> > We do allow fields to be deleted, which is a *non-compatible*
>> > evolution, but they MUST have been marked as deprecated for
>> > 2 cycles first.
>> 
>> Good point.
>> 
>> > I'd say sorting required vs optional arguments is doomed as
>> > a strategy. Stuff that is mandatory today can be made optional
>> > tomorrow and I think that's reasonable to want todo as we
>> > evolve an API design.
>> 
>> Also a good point. Python requires all mandatory arguments precede all
>> optional ones, so you're probably right that in order to maximize
>> cross-version compatibility, keyword-only arguments for *all*
>> arguments both mandatory and optional is the only real way to fly.
>> 
>> I think this might cause problems for Marc-Andre in rust/dbus land,
>> but it's tractable in Python. I am unclear on the ramifications for
>> golang. (Victor, Marc-Andre, any input here?)
>
> Well as a general point, if we consider usage outside of
> qemu.git, I'm far from convinced that generating code from
> the schema as it exists today is going to be broadly usable
> enough to make it worthwhile.
>
> The problem is precisely that the code that is generated is
> only ensured to work with the specific version of QEMU that
> it was generated from. The key problem here is the removal
> of features after deprecation.  That removal is fine if you
> only ever need an API to talk to the current QEMU, or only
> need to be able to introspect the current QEMU.
>
> Management apps are likely to want to write code that works
> with more than 1 version of QEMU, and be able to decide
> whether they provide the params needed by the old command
> or the new command.   The introspection data lets them
> make the decision about which command needs to be invoked,
> but it can't be used to generate the code needed for the
> old command.
>
> I don't know how exactly you're dealing with the Python
> mapping. If you're literally statically generating Python
> code you'll face this same problem. If on the other hand
> you've just got a stub object that does dynamic dispatch
> then it can dynamically adapt at runtime to work with
> whatever version of the schema it queried from the QEMU
> it is talking to. I'm hoping you're doing the latter
> for this reason.
>
> One strategy for compiled languages is to generate multiple
> copies of the APIs, one for each QEMU version. This could
> be made to work but I feel it is pretty horrific as an
> approach.  Libvirt has one set of client APIs that we've
> extended over time so they can be used to call both old
> and new variants of commands - we just need to use the
> introspected schema to decide which to use.
>
> To make the latter viable for generating compiled code
> though, we need to change how we deal with removals, by
> essentially never removing things at all. Instead we
> would need some way to express that "field A" or "type T"
> is not present after some point in time / release.
>
> Overall I don't think we're really in a position to use
> compiled auto generated bindings, except for QMP clients
> that are released in lockstep with specific QEMU versions,
> and I don't think lockstep releases are a viable strategy
> in general.

You described two choices.  I believe there's a third one.

A management application can choose to target a single QEMU version, and
require lockstep upgrade, i.e. upgrading QEMU requires upgrading to the
matching management application, and vice versa.  This is quite a hassle
for users.

The other extreme is to target a range of QEMU versions that is so wide
that the management application has to deal both with old and new
interfaces.  QEMU provides schema introspection to help with that, and
libvirt makes use of it.

A third choice is to target the narrow range of QEMU versions where the
deprecation policy protects you.  If a management application release
for QEMU version N uses no deprecated interfaces, it should be good up
to version N+2.  That's a full year.  Less wiggle room than libvirt
provides.  Whether the extra wiggle room is worth the extra effort is
for the management application developers to decide.

Note that this *can* use bindings generated for version N.  These
bindings talk wire format version N to QEMU, which QEMU up to version
N+2 must understand as per deprecation policy.

The difference between the first and the last choice is some extra
analysis and testing: you have to ensure no uses of deprecated
interfaces are left (QEMU provides -compat for that since v6.0), and you
have to keep testing with new QEMU releases (preferably *before* they
come out), to guard against deprecation policy violations slipping
through.  Same testing as for the second choice, just for fewer QEMU
releases.

>> > It sounds like you need a wrapper type.  This StrOrNull scenario
>> > is QMP's "alternate" type IIUC, but you're trying to avoid
>> > expressing the existance fo the "alternate" type in the API
>> 
>> Yes. This is a very clean way to type it, but it is a little more
>> laborious for the user to have to remember to wrap certain strings in
>> a special constructor first. Still, this is a good trick that I hadn't
>> considered. I'll keep it in mind for future experiments.
>
> Bear in mind that this situation is pretty rare, so I don't think
> the user is especially burdened by needing an extra level of
> indirection for using 'alternate' types
>
> $ git grep "'alternate'" qapi
> qapi/block-core.json:{ 'alternate': 'BlockDirtyBitmapMergeSource',
> qapi/block-core.json:{ 'alternate': 'Qcow2OverlapChecks',
> qapi/block-core.json:{ 'alternate': 'BlockdevRef',
> qapi/block-core.json:{ 'alternate': 'BlockdevRefOrNull',
> qapi/common.json:{ 'alternate': 'StrOrNull',
>
> $ git grep "'StrOrNull'" qapi
> qapi/block-core.json:             'iothread': 'StrOrNull',
> qapi/common.json:{ 'alternate': 'StrOrNull',
> qapi/migration.json:            '*tls-creds': 'StrOrNull',
> qapi/migration.json:            '*tls-hostname': 'StrOrNull',
> qapi/migration.json:            '*tls-authz': 'StrOrNull',

Yes.

Apropos StrOrNull.  The natural C binding would be a nullable const *
(plain str is *not* nullable).  But StrOrNull is too rare to bother.