qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: John Snow <jsnow@redhat.com>
To: Wainer dos Santos Moschetta <wainersm@redhat.com>, qemu-devel@nongnu.org
Cc: ehabkost@redhat.com, crosa@redhat.com
Subject: Re: [PATCH 2/5] python/qemu: Delint the qmp module
Date: Wed, 8 Jan 2020 19:17:10 -0500	[thread overview]
Message-ID: <415f6e37-5d57-1a00-ef9f-80472b85e0c6@redhat.com> (raw)
In-Reply-To: <20191227134101.244496-3-wainersm@redhat.com>



On 12/27/19 8:40 AM, Wainer dos Santos Moschetta wrote:
> This clean up the pylint-3 report on qmp:
> 
> ************* Module qemu.qmp
> python/qemu/qmp.py:1:0: C0111: Missing module docstring (missing-docstring)
> python/qemu/qmp.py:17:0: C0111: Missing class docstring (missing-docstring)
> python/qemu/qmp.py:21:0: C0111: Missing class docstring (missing-docstring)
> python/qemu/qmp.py:25:0: C0111: Missing class docstring (missing-docstring)
> python/qemu/qmp.py:29:0: C0111: Missing class docstring (missing-docstring)
> python/qemu/qmp.py:33:0: C0111: Missing class docstring (missing-docstring)
> python/qemu/qmp.py:33:0: R0205: Class 'QEMUMonitorProtocol' inherits from object, can be safely removed from bases in python3 (useless-object-inheritance)
> python/qemu/qmp.py:80:4: R1710: Either all return statements in a function should return an expression, or none of them should. (inconsistent-return-statements)
> python/qemu/qmp.py:131:4: R1710: Either all return statements in a function should return an expression, or none of them should. (inconsistent-return-statements)
> python/qemu/qmp.py:159:4: R1710: Either all return statements in a function should return an expression, or none of them should. (inconsistent-return-statements)
> python/qemu/qmp.py:245:4: C0111: Missing method docstring (missing-docstring)
> python/qemu/qmp.py:249:4: C0111: Missing method docstring (missing-docstring)
> python/qemu/qmp.py:252:4: C0111: Missing method docstring (missing-docstring)
> python/qemu/qmp.py:255:4: C0111: Missing method docstring (missing-docstring)
> 
> Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>

ACK, thanks for the delinting work here. It will be nice to get our
python code in such a shape that it *could* be packaged if we want to.

Like, if we wanted to publish a standalone QMP client, for instance...

Reviewed-by: John Snow <jsnow@redhat.com>

> ---
>  python/qemu/qmp.py | 51 +++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 41 insertions(+), 10 deletions(-)
> 
> diff --git a/python/qemu/qmp.py b/python/qemu/qmp.py
> index 8c6c9847d0..f4e04a6683 100644
> --- a/python/qemu/qmp.py
> +++ b/python/qemu/qmp.py
> @@ -1,5 +1,4 @@
> -# QEMU Monitor Protocol Python class
> -#
> +""" QEMU Monitor Protocol Python class """
>  # Copyright (C) 2009, 2010 Red Hat Inc.
>  #
>  # Authors:
> @@ -15,22 +14,34 @@ import logging
>  
>  
>  class QMPError(Exception):
> -    pass
> +    """
> +    QMP base exception
> +    """
>  
>  
>  class QMPConnectError(QMPError):
> -    pass
> +    """
> +    QMP connection exception
> +    """
>  
>  
>  class QMPCapabilitiesError(QMPError):
> -    pass
> +    """
> +    QMP negotiate capabilities exception
> +    """
>  
>  
>  class QMPTimeoutError(QMPError):
> -    pass
> +    """
> +    QMP timeout exception
> +    """
>  
>  
> -class QEMUMonitorProtocol(object):
> +class QEMUMonitorProtocol:
> +    """
> +    Provide an API to connect to QEMU via QEMU Monitor Protocol (QMP) and then
> +    allow to handle commands and events.
> +    """
>  
>      #: Logger object for debugging messages
>      logger = logging.getLogger('QMP')
> @@ -81,7 +92,7 @@ class QEMUMonitorProtocol(object):
>          while True:
>              data = self.__sockfile.readline()
>              if not data:
> -                return
> +                return None
>              resp = json.loads(data)
>              if 'event' in resp:
>                  self.logger.debug("<<< %s", resp)
> @@ -132,7 +143,7 @@ class QEMUMonitorProtocol(object):
>          """
>          Connect to the QMP Monitor and perform capabilities negotiation.
>  
> -        @return QMP greeting dict
> +        @return QMP greeting dict, or None if negotiate is false
>          @raise OSError on socket connection errors
>          @raise QMPConnectError if the greeting is not received
>          @raise QMPCapabilitiesError if fails to negotiate capabilities
> @@ -141,6 +152,7 @@ class QEMUMonitorProtocol(object):
>          self.__sockfile = self.__sock.makefile()
>          if negotiate:
>              return self.__negotiate_capabilities()
> +        return None
>  
>      def accept(self):
>          """
> @@ -169,7 +181,7 @@ class QEMUMonitorProtocol(object):
>              self.__sock.sendall(json.dumps(qmp_cmd).encode('utf-8'))
>          except OSError as err:
>              if err.errno == errno.EPIPE:
> -                return
> +                return None
>              raise err
>          resp = self.__json_read()
>          self.logger.debug("<<< %s", resp)
> @@ -243,14 +255,33 @@ class QEMUMonitorProtocol(object):
>          self.__events = []
>  
>      def close(self):
> +        """
> +        Close the socket and socket file.
> +        """
>          self.__sock.close()
>          self.__sockfile.close()
>  
>      def settimeout(self, timeout):
> +        """
> +        Set the socket timeout.
> +
> +        @param timeout (float): timeout in seconds, or None.
> +        @note This is a wrap around socket.settimeout
> +        """
>          self.__sock.settimeout(timeout)
>  
>      def get_sock_fd(self):
> +        """
> +        Get the socket file descriptor.
> +
> +        @return The file descriptor number.
> +        """
>          return self.__sock.fileno()
>  
>      def is_scm_available(self):
> +        """
> +        Check if the socket allows for SCM_RIGHTS.
> +
> +        @return True if SCM_RIGHTS is available, otherwise False.
> +        """
>          return self.__sock.family == socket.AF_UNIX
> 

-- 
—js



  reply	other threads:[~2020-01-09  0:18 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-27 13:40 [PATCH 0/5] python/qemu: qmp: Fix, delint and improvements Wainer dos Santos Moschetta
2019-12-27 13:40 ` [PATCH 1/5] python/qemu: qmp: Replace socket.error with OSError Wainer dos Santos Moschetta
2020-01-09  0:15   ` John Snow
2019-12-27 13:40 ` [PATCH 2/5] python/qemu: Delint the qmp module Wainer dos Santos Moschetta
2020-01-09  0:17   ` John Snow [this message]
2020-01-30 22:38   ` Philippe Mathieu-Daudé
2019-12-27 13:40 ` [PATCH 3/5] python/qemu: qmp: Make accept()'s timeout configurable Wainer dos Santos Moschetta
2019-12-27 19:07   ` Philippe Mathieu-Daudé
2020-01-09  0:18   ` John Snow
2020-01-30 22:35     ` Philippe Mathieu-Daudé
2019-12-27 13:41 ` [PATCH 4/5] python/qemu: qmp: Make QEMUMonitorProtocol a context manager Wainer dos Santos Moschetta
2020-01-09  0:23   ` John Snow
2020-01-29 20:07     ` Wainer dos Santos Moschetta
2019-12-27 13:41 ` [PATCH 5/5] python/qemu: qmp: Remove unnused attributes Wainer dos Santos Moschetta
2020-01-09  0:23   ` John Snow
2020-01-30 22:40   ` Philippe Mathieu-Daudé
2020-01-30 22:41 ` [PATCH 0/5] python/qemu: qmp: Fix, delint and improvements Philippe Mathieu-Daudé
2020-01-31 13:08   ` Wainer dos Santos Moschetta

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=415f6e37-5d57-1a00-ef9f-80472b85e0c6@redhat.com \
    --to=jsnow@redhat.com \
    --cc=crosa@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=wainersm@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).