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