[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 05/16] python/qmp.py: add casts to JSON deserialization
From: |
John Snow |
Subject: |
Re: [PATCH v2 05/16] python/qmp.py: add casts to JSON deserialization |
Date: |
Thu, 4 Jun 2020 14:33:51 -0400 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 |
On 6/4/20 9:49 AM, Kevin Wolf wrote:
> Am 02.06.2020 um 23:45 hat John Snow geschrieben:
>> mypy and python type hints are not powerful enough to properly describe
>> JSON messages in Python 3.6. The best we can do, generally, is describe
>> them as Dict[str, Any].
>>
>> Add casts to coerce this type for static analysis; but do NOT enforce
>> this type at runtime in any way.
>>
>> Note: Python 3.8 adds a TypedDict construct which allows for the
>> description of more arbitrary Dictionary shapes. There is a third-party
>> module, "Pydantic", which is compatible with 3.6 that can be used
>> instead of the JSON library that parses JSON messages to fully-typed
>> Python objects, and may be preferable in some cases.
>>
>> (That is well beyond the scope of this commit or series.)
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>> python/qemu/qmp.py | 8 ++++++--
>> 1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/python/qemu/qmp.py b/python/qemu/qmp.py
>> index ef3c919b76c..5f3558e3066 100644
>> --- a/python/qemu/qmp.py
>> +++ b/python/qemu/qmp.py
>> @@ -13,6 +13,7 @@
>> import logging
>> from typing import (
>> Any,
>> + cast,
>> Dict,
>> Optional,
>> TextIO,
>> @@ -130,7 +131,10 @@ def __json_read(self, only_event=False):
>> data = self.__sockfile.readline()
>> if not data:
>> return None
>> - resp = json.loads(data)
>> + # By definition, any JSON received from QMP is a QMPMessage,
>> + # and we are asserting only at static analysis time that it
>> + # has a particular shape.
>> + resp = cast(QMPMessage, json.loads(data))
>
> Instead of casting, you can just specify the variable type:
>
> resp: QMPMessage = json.loads(data)
>
> I don't think that json.loads() will actually return something other
> than Any anytime soon, but it's generally nicer to avoid casts and if it
> eventually does change, we'll get the type check instead of silencing
> it.
>
Nice, that works with --strict too. Done.
>> if 'event' in resp:
>> self.logger.debug("<<< %s", resp)
>> self.__events.append(resp)
>> @@ -262,7 +266,7 @@ def command(self, cmd, **kwds):
>> ret = self.cmd(cmd, kwds)
>> if 'error' in ret:
>> raise QMPResponseError(ret)
>> - return ret['return']
>> + return cast(QMPReturnValue, ret['return'])
>
> This one can't be easily avoided, though.
>
Sadly not -- we could conceivably make use of e.g. pydantic to create
arbitrarily nuanced/strict structure definitions that are actually
validated at runtime, but I will save that for a future series.
--js
- [PATCH v2 00/16] python: add mypy support to python/qemu, John Snow, 2020/06/02
- [PATCH v2 02/16] iotests.py: use qemu.qmp type aliases, John Snow, 2020/06/02
- [PATCH v2 01/16] python/qmp.py: Define common types, John Snow, 2020/06/02
- [PATCH v2 03/16] python/qmp.py: re-absorb MonitorResponseError, John Snow, 2020/06/02
- [PATCH v2 04/16] python/qmp.py: Do not return None from cmd_obj, John Snow, 2020/06/02
- [PATCH v2 05/16] python/qmp.py: add casts to JSON deserialization, John Snow, 2020/06/02
- [PATCH v2 06/16] python/qmp.py: add QMPProtocolError, John Snow, 2020/06/02
- [PATCH v2 07/16] python/machine.py: Fix monitor address typing, John Snow, 2020/06/02
- [PATCH v2 09/16] python/machine.py: Don't modify state in _base_args(), John Snow, 2020/06/02
- [PATCH v2 08/16] python/machine.py: reorder __init__, John Snow, 2020/06/02
- [PATCH v2 10/16] python/machine.py: Handle None events in event_wait, John Snow, 2020/06/02
- [PATCH v2 12/16] python/machine.py: Add _qmp access shim, John Snow, 2020/06/02
- [PATCH v2 11/16] python/machine.py: use qmp.command, John Snow, 2020/06/02
- [PATCH v2 14/16] python/qemu: make 'args' style arguments immutable, John Snow, 2020/06/02