[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 05/10] qemu.py: Use custom exceptions rather
From: |
Lukáš Doktor |
Subject: |
Re: [Qemu-devel] [PATCH v2 05/10] qemu.py: Use custom exceptions rather than Exception |
Date: |
Wed, 26 Jul 2017 06:39:28 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 |
Dne 25.7.2017 v 18:06 Eduardo Habkost napsal(a):
> On Tue, Jul 25, 2017 at 05:09:46PM +0200, Lukáš Doktor wrote:
>> The naked Exception should not be widely used. It makes sense to be a
>> bit more specific and use better-suited custom exceptions. As a benefit
>> we can store the full reply in the exception in case someone needs it
>> when catching the exception.
>>
>> Signed-off-by: Lukáš Doktor <address@hidden>
>> Reviewed-by: Eduardo Habkost <address@hidden>
>> ---
>> scripts/qemu.py | 17 +++++++++++++++--
>> 1 file changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/scripts/qemu.py b/scripts/qemu.py
>> index 5948e19..e6df54c 100644
>> --- a/scripts/qemu.py
>> +++ b/scripts/qemu.py
>> @@ -19,6 +19,19 @@ import subprocess
>> import qmp.qmp
>>
>>
>> +class MonitorResponseError(qmp.qmp.QMPError):
>> + '''
>> + Represents erroneous QMP monitor reply
>> + '''
>> + def __init__(self, reply):
>> + try:
>> + desc = reply["error"]["desc"]
>> + except KeyError:
>> + desc = reply
> ^^^ (*)
>> + super(MonitorResponseError, self).__init__(repr(desc))
>
> This will end up calling Exception.__init__. I previously
> suggested repr(desc) above(*) because I didn't know what happened
> when the Exception.__init__ argument is not a string.
>
> I couldn't find any documentation on the right argument types for
> Exception.__init__. The examples in the Python documentation[1]
> don't call Exception.__init__ at all: they simply implement
> __str__().
>
> However, based on my testing and on the BaseException
> documentation[2], I believe repr() isn't really necessary:
>
> "If str() or unicode() is called on an instance of this class,
> the representation of the argument(s) to the instance are
> returned, or the empty string when there were no arguments."
>
> So your previous version was good, already.
>
> But maybe we could do this instead, to make the constructor as
> simple as possible:
>
> class MonitorResponseError(qmp.qmp.QMPError):
> def __init__(self, reply):
> self.reply = reply
>
> def __str__(self):
> return self.reply.get('error', {}).get('desc', repr(self.reply))
>
Well I know I can implement custom `__str__` class, but the default one simply
stores the argument as string and reports it, so for simple strings I usually
go with super call and with complex ones with custom `__str__`. Anyway if this
suits this project style better I'll change it in v2.
Lukáš
>
> [1] https://docs.python.org/2/tutorial/errors.html#user-defined-exceptions
> [2] https://docs.python.org/2/library/exceptions.html#exceptions.BaseException
>
>> + self.reply = reply
>> +
>> +
>> class QEMUMachine(object):
>> '''A QEMU VM'''
>>
>> @@ -197,9 +210,9 @@ class QEMUMachine(object):
>> '''
>> reply = self.qmp(cmd, conv_keys, **args)
>> if reply is None:
>> - raise Exception("Monitor is closed")
>> + raise qmp.qmp.QMPError("Monitor is closed")
>> if "error" in reply:
>> - raise Exception(reply["error"]["desc"])
>> + raise MonitorResponseError(reply)
>> return reply["return"]
>>
>> def get_qmp_event(self, wait=False):
>> --
>> 2.9.4
>>
>
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH v2 00/10] qemu.py: Pylint/style fixes, Lukáš Doktor, 2017/07/25
- [Qemu-devel] [PATCH v2 01/10] qemu.py: Pylint/style fixes, Lukáš Doktor, 2017/07/25
- [Qemu-devel] [PATCH v2 02/10] qemu|qtest: Avoid dangerous arguments, Lukáš Doktor, 2017/07/25
- [Qemu-devel] [PATCH v2 04/10] qemu.py: Simplify QMP key-conversion, Lukáš Doktor, 2017/07/25
- [Qemu-devel] [PATCH v2 03/10] qemu.py: Use iteritems rather than keys(), Lukáš Doktor, 2017/07/25
- [Qemu-devel] [PATCH v2 05/10] qemu.py: Use custom exceptions rather than Exception, Lukáš Doktor, 2017/07/25
- [Qemu-devel] [PATCH v2 06/10] qmp.py: Couple of pylint/style fixes, Lukáš Doktor, 2017/07/25
- [Qemu-devel] [PATCH v2 07/10] qmp.py: Use object-based class for QEMUMonitorProtocol, Lukáš Doktor, 2017/07/25
- [Qemu-devel] [PATCH v2 08/10] qmp.py: Avoid "has_key" usage, Lukáš Doktor, 2017/07/25
- [Qemu-devel] [PATCH v2 09/10] qmp.py: Avoid overriding a builtin object, Lukáš Doktor, 2017/07/25
[Qemu-devel] [PATCH v2 10/10] qtest.py: Few pylint/style fixes, Lukáš Doktor, 2017/07/25