Re: [Qemu-devel] [PATCH] fixup! scripts: Remove debug parameter from QEMUMonitorProtocol

["Lukáš Doktor" <ldoktor@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2691 bytes --]

Dne 27.9.2017 v 15:44 Eduardo Habkost napsal(a):
> On Wed, Sep 27, 2017 at 09:33:21PM +0800, Fam Zheng wrote:
>> On Wed, 09/27 10:03, Eduardo Habkost wrote:
>>> @@ -51,7 +54,6 @@ class QEMUMonitorProtocol(object):
>>>          """
>>>          self.__events = []
>>>          self.__address = address
>>> -        self._debug = debug
>>
>> Should you also drop the debug parameter from the method?
>>
>>>          self.__sock = self.__get_sock()
>>>          self.__sockfile = None
>>>          if server:
>>> @@ -83,8 +85,7 @@ class QEMUMonitorProtocol(object):
>>>                  return
>>>              resp = json.loads(data)
>>>              if 'event' in resp:
>>> -                if self._debug:
>>> -                    print >>sys.stderr, "QMP:<<< %s" % resp

This is the only user of `sys` import, please remove it as well. Apart from this it looks good, although you might consider using `__name__` instead of hardcoded `QMP` in `logger = logging.getLogger(__name__)` for the sake of consistency (people might expect it to correlate with the module name).

Lukáš

>>> +                self.logger.debug("<<< %s", resp)
>>>                  self.__events.append(resp)
>>>                  if not only_event:
>>>                      continue
>>> @@ -164,8 +165,7 @@ class QEMUMonitorProtocol(object):
>>>          @return QMP response as a Python dict or None if the connection has
>>>                  been closed
>>>          """
>>> -        if self._debug:
>>> -            print >>sys.stderr, "QMP:>>> %s" % qmp_cmd
>>> +        self.logger.debug("<<< %s", qmp_cmd)
>>
>> This should be ">>> %s".
>>
> 
> Fixed.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  scripts/qmp/qmp.py | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/qmp/qmp.py b/scripts/qmp/qmp.py
> index be79d7aa80..369d9fef39 100644
> --- a/scripts/qmp/qmp.py
> +++ b/scripts/qmp/qmp.py
> @@ -40,7 +40,7 @@ class QEMUMonitorProtocol(object):
>      #: Socket's timeout
>      timeout = socket.timeout
>  
> -    def __init__(self, address, server=False, debug=False):
> +    def __init__(self, address, server=False):
>          """
>          Create a QEMUMonitorProtocol class.
>  
> @@ -165,7 +165,7 @@ class QEMUMonitorProtocol(object):
>          @return QMP response as a Python dict or None if the connection has
>                  been closed
>          """
> -        self.logger.debug("<<< %s", qmp_cmd)
> +        self.logger.debug(">>> %s", qmp_cmd)
>          try:
>              self.__sock.sendall(json.dumps(qmp_cmd))
>          except socket.error as err:
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 502 bytes --]