qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH RFC 26/32] python//machine.py: use qmp.command


From: Kevin Wolf
Subject: Re: [PATCH RFC 26/32] python//machine.py: use qmp.command
Date: Tue, 2 Jun 2020 12:18:55 +0200

Am 29.05.2020 um 02:18 hat John Snow geschrieben:
> 
> [...]
> 
> >  
> > -    def qmp(self, cmd, conv_keys=True, **args):
> > -        """
> > -        Invoke a QMP command and return the response dict
> > -        """
> > +    @classmethod
> > +    def _qmp_args(cls, _conv_keys: bool = True, **args: Any) -> Dict[str, 
> > Any]:
> >          qmp_args = dict()
> >          for key, value in args.items():
> > -            if conv_keys:
> > +            if _conv_keys:
> >                  qmp_args[key.replace('_', '-')] = value
> >              else:
> >                  qmp_args[key] = value
> > +        return qmp_args
> >  
> > +    def qmp(self, cmd: str,
> > +            conv_keys: bool = True,
> > +            **args: Any) -> QMPMessage:
> 
> This creates an interesting problem with iotests 297:
> 
> 
> -Success: no issues found in 1 source file
> +iotests.py:563: error: Argument 2 to "qmp" of "QEMUMachine" has
> incompatible type "**Dict[str, str]"; expected "bool"
> +Found 1 error in 1 file (checked 1 source file)
> 
> 
> def hmp(self, command_line: str, use_log: bool = False) -> QMPResponse:
>     cmd = 'human-monitor-command'
>     kwargs = {'command-line': command_line}
>     if use_log:
>         return self.qmp_log(cmd, **kwargs)
>     else:
>         return self.qmp(cmd, **kwargs)
> 
> It seems like mypy is unable to understand that we are passing keyword
> arguments, and instead believes we're passing something to the conv_keys
> parameter.
> 
> (Is this a bug...?)

It took some experimenting, but in the end, I think the logic behind it
is reasonable enough. If you have kwargs: Dict[str, T] and pass it as
**kwargs, mypy checks that passing T to _every_ keyword argument is fine
because generally speaking you have to expect that the name of any
argument could be present in kwargs. (In the specific example, of
course, we know all the keys, but mypy doesn't look at the content of
the dict.)

With this in mind, getting rid of the error is very simple because you
only need to make sure that T is Any instead of str:

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 7eb714b8e5..0258f1359a 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -556,7 +556,7 @@ class VM(qtest.QEMUQtestMachine):
 
     def hmp(self, command_line: str, use_log: bool = False) -> QMPResponse:
         cmd = 'human-monitor-command'
-        kwargs = {'command-line': command_line}
+        kwargs: Dict[str, Any] = {'command-line': command_line}
         if use_log:
             return self.qmp_log(cmd, **kwargs)
         else:

Kevin




reply via email to

[Prev in Thread] Current Thread [Next in Thread]