scripts/qmp/qmp.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
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
> > + 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:
--
2.13.5
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: >
On Thu, Sep 28, 2017 at 11:33:57AM +0200, Lukáš Doktor wrote: > 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). I will remove 'import sys' in v2, but I disagree about using the module name: prefixing them with "QMP" makes them more readable and familiar than using "qmp.qmp". Thanks! > > 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: > > > > -- Eduardo
© 2016 - 2024 Red Hat, Inc.