Re: [Qemu-devel] [PATCH v6 10/27] qapi: factor out checking for keys

[Marc-André Lureau]

On Wed, Dec 5, 2018 at 8:26 PM Markus Armbruster <address@hidden> wrote:
>
> Marc-André Lureau <address@hidden> writes:
>
> > Introduce a new helper function to check if the given keys are known,
> > and if mandatory keys are present. The function will be reused in
> > other places in the following code changes.
> >
> > Signed-off-by: Marc-André Lureau <address@hidden>
> > ---
> >  scripts/qapi/common.py | 20 +++++++++++++-------
> >  1 file changed, 13 insertions(+), 7 deletions(-)
> >
> > diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> > index a353670079..8313c478c4 100644
> > --- a/scripts/qapi/common.py
> > +++ b/scripts/qapi/common.py
> > @@ -873,6 +873,17 @@ def check_struct(expr, info):
> >                 allow_metas=['struct'])
> >
> >
> > +def check_known_keys(info, source, keys, required, optional):
> > +    for key in keys:
> > +        if key not in required and key not in optional:
> > +            raise QAPISemError(info, "Unknown key '%s' in %s" % (key, 
> > source))
> > +
> > +    for key in required:
> > +        if key not in keys:
> > +            raise QAPISemError(info, "Key '%s' is missing from %s"
> > +                               % (key, source))
> > +
> > +
> >  def check_keys(expr_elem, meta, required, optional=[]):
> >      expr = expr_elem['expr']
> >      info = expr_elem['info']
> > @@ -880,10 +891,9 @@ def check_keys(expr_elem, meta, required, optional=[]):
> >      if not isinstance(name, str):
> >          raise QAPISemError(info, "'%s' key must have a string value" % 
> > meta)
> >      required = required + [meta]
> > +    source = "%s '%s'" % (meta, name)
> > +    check_known_keys(info, source, list(expr.keys()), required, optional)
>
> Sure you need list() here?

I don't know, that's what python-modernize suggests.

Also the top answer from:
https://stackoverflow.com/questions/16819222/how-to-return-dictionary-keys-as-a-list-in-python#16819250

Do you mind if I leave the list() ? (mostly for python-modernize to be happy)


>
> >      for (key, value) in expr.items():
> > -        if key not in required and key not in optional:
> > -            raise QAPISemError(info, "Unknown key '%s' in %s '%s'"
> > -                               % (key, meta, name))
> >          if key in ['gen', 'success-response'] and value is not False:
> >              raise QAPISemError(info,
> >                                 "'%s' of %s '%s' should only use false 
> > value"
> > @@ -895,10 +905,6 @@ def check_keys(expr_elem, meta, required, optional=[]):
> >                                 % (key, meta, name))
> >          if key == 'if':
> >              check_if(expr, info)
> > -    for key in required:
> > -        if key not in expr:
> > -            raise QAPISemError(info, "Key '%s' is missing from %s '%s'"
> > -                               % (key, meta, name))
> >
> >
> >  def check_exprs(exprs):