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 <ldok...@redhat.com>
>> Reviewed-by: Eduardo Habkost <ehabk...@redhat.com>
>> ---
>> 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
