On Thu, Jul 20, 2017 at 06:28:09PM +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>
> ---
> scripts/qemu.py | 17 +++++++++++++++--
> 1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/qemu.py b/scripts/qemu.py
> index 5948e19..2bd522f 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__(desc)
> + self.reply = reply
This would require every user of self.reply to first check if
it's a string or dictionary. All because of the "Monitor is
closed" case below:
> +
> +
> 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")
Should we really use the same exception type for this, if it's
not really a QMP monitor error reply, and won't even have a real
QMP reply in self.reply?
> 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
>
--
Eduardo
