On Mon, Mar 06, 2017 at 02:43:50PM +0300, Joannah Nanjekye wrote: > From: Joannah Najekye <nanjekyejoan...@gmail.com> > > Signed-off-by: Joannah Nanjekye <nanjekyejoan...@gmail.com> > ---
Text below '---' is not included as part of the commit description. I think the following lines should be included, they describe the purpose of the patch. Please the text above '---'. > > The patch provides python 3 support for one of the scripts in scripts/qmp > that is to say qmp.py. This is not a port to python 3 but rather the patch > ensures that the script runs fine for both python 2 and 3. > > Minimum Python Versions supported: > > Python 2 : python 2.6 + > Python 3 : python 3.3 + > > The two new imports future and builtins introduced refer to the future > pip-installable package on PyPI. PyPI packages can be used if necessary but dependencies should be minimized. Comments on the new dependencies below: > scripts/qmp/qmp.py | 22 ++++++++++++---------- > 1 file changed, 12 insertions(+), 10 deletions(-) > > diff --git a/scripts/qmp/qmp.py b/scripts/qmp/qmp.py > index 62d3651..6dcb870 100644 > --- a/scripts/qmp/qmp.py > +++ b/scripts/qmp/qmp.py > @@ -7,7 +7,9 @@ > # > # This work is licensed under the terms of the GNU GPL, version 2. See > # the COPYING file in the top-level directory. > - > +from __future__ import absolute_import, print_function, unicode_literals > +from future.utils import raise_from Why is raise_from() needed? My understanding is that it provides exception chaining, which this code doesn't use. Therefore plain 'raise' is enough. The dependency can be dropped. > +from builtins import str > import json > import errno > import socket > @@ -57,12 +59,12 @@ class QEMUMonitorProtocol: > def __negotiate_capabilities(self): > greeting = self.__json_read() > if greeting is None or not greeting.has_key('QMP'): > - raise QMPConnectError > + raise_from(QMPConnectError()) > # Greeting seems ok, negotiate capabilities > resp = self.cmd('qmp_capabilities') > if "return" in resp: > return greeting > - raise QMPCapabilitiesError > + raise_from(QMPCapabilitiesError()) > > def __json_read(self, only_event=False): > while True: > @@ -72,7 +74,7 @@ class QEMUMonitorProtocol: > resp = json.loads(data) > if 'event' in resp: > if self._debug: > - print >>sys.stderr, "QMP:<<< %s" % resp > + print(u"QMP:<<< %s" % str(resp), file=sys.stderr) Why is str() necessary? resp is a dict and the %s format specifier already produces a string representation. The same applies to the str(qmp_cmd) and str(resp) changes below. > self.__events.append(resp) > if not only_event: > continue > @@ -111,11 +113,11 @@ class QEMUMonitorProtocol: > try: > ret = self.__json_read(only_event=True) > except socket.timeout: > - raise QMPTimeoutError("Timeout waiting for event") > + raise_from(QMPTimeoutError("Timeout waiting for event")) > except: > - raise QMPConnectError("Error while reading from socket") > + raise_from(QMPConnectError("Error while reading from > socket")) > if ret is None: > - raise QMPConnectError("Error while reading from socket") > + raise_from(QMPConnectError("Error while reading from > socket")) > self.__sock.settimeout(None) > > def connect(self, negotiate=True): > @@ -155,16 +157,16 @@ class QEMUMonitorProtocol: > been closed > """ > if self._debug: > - print >>sys.stderr, "QMP:>>> %s" % qmp_cmd > + print(u"QMP:>>> %s" % str(qmp_cmd), file=sys.stderr) > try: > self.__sock.sendall(json.dumps(qmp_cmd)) > except socket.error as err: > if err[0] == errno.EPIPE: > return > - raise socket.error(err) > + raise_from(socket.error(), err) > resp = self.__json_read() > if self._debug: > - print >>sys.stderr, "QMP:<<< %s" % resp > + print(u"QMP:<<< %s" % str(resp), file=sys.stderr) > return resp > > def cmd(self, name, args=None, id=None): > -- > 2.7.4 >
signature.asc
Description: PGP signature