Re: [PATCH v2 22/25] python/aqmp: copy qmp docstrings to qemu.aqmp.legacy

[Beraldo Leal]

On Wed, Dec 15, 2021 at 02:39:36PM -0500, John Snow wrote:
> Copy the docstrings out of qemu.qmp, adjusting them as necessary to
> more accurately reflect the current state of this class.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  python/qemu/aqmp/legacy.py | 110 ++++++++++++++++++++++++++++++++++---
>  1 file changed, 102 insertions(+), 8 deletions(-)
> 
> diff --git a/python/qemu/aqmp/legacy.py b/python/qemu/aqmp/legacy.py
> index 8f38e7d912..6c250cd46a 100644
> --- a/python/qemu/aqmp/legacy.py
> +++ b/python/qemu/aqmp/legacy.py
> @@ -1,9 +1,23 @@
>  """
> -Sync QMP Wrapper
> +(Legacy) Sync QMP Wrapper
>  
> -This class pretends to be qemu.qmp.QEMUMonitorProtocol.
> +This module provides the `QEMUMonitorProtocol` class, which is a
> +synchronous wrapper around `QMPClient`.
> +
> +Its design closely resembles that of the original QEMUMonitorProtocol
> +class, originally written by Luiz Capitulino.
>  """
>  
> +# Copyright (C) 2009, 2010, 2021 Red Hat Inc.
> +#
> +# Authors:
> +#  Luiz Capitulino <lcapitulino@redhat.com>
> +#  John Snow <jsnow@redhat.com>
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2.
> +# See the COPYING file in the top-level directory.
> +
> +
>  import asyncio
>  from types import TracebackType
>  from typing import (
> @@ -39,9 +53,6 @@
>  # {} is the QMPReturnValue.
>  
>  
> -# pylint: disable=missing-docstring
> -
> -
>  class QMPBadPortError(QMPError):
>      """
>      Unable to parse socket address: Port was non-numerical.
> @@ -49,6 +60,21 @@ class QMPBadPortError(QMPError):
>  
>  
>  class QEMUMonitorProtocol:
> +    """
> +    Provide an API to connect to QEMU via QEMU Monitor Protocol (QMP)
> +    and then allow to handle commands and events.
> +
> +    :param address:  QEMU address, can be either a unix socket path (string)
> +                     or a tuple in the form ( address, port ) for a TCP
> +                     connection
> +    :param server:   Deprecated, ignored. (See 'accept')
> +    :param nickname: Optional nickname used for logging.
> +
> +    ..note::
> +        No connection is established during `__init__`, this is done by
> +        the `connect()` or `accept()` methods.
> +    """
> +
>      def __init__(self, address: SocketAddrT,
>                   server: bool = False,  # pylint: disable=unused-argument
>                   nickname: Optional[str] = None):
> @@ -108,6 +134,12 @@ def parse_address(cls, address: str) -> SocketAddrT:
>          return address
>  
>      def connect(self, negotiate: bool = True) -> Optional[QMPMessage]:
> +        """
> +        Connect to the QMP Monitor and perform capabilities negotiation.
> +
> +        :return: QMP greeting dict, or None if negotiate is false
> +        :raise ConnectError: on connection errors
> +        """
>          self._aqmp.await_greeting = negotiate
>          self._aqmp.negotiate = negotiate
>  
> @@ -117,6 +149,16 @@ def connect(self, negotiate: bool = True) -> 
> Optional[QMPMessage]:
>          return self._get_greeting()
>  
>      def accept(self, timeout: Optional[float] = 15.0) -> QMPMessage:
> +        """
> +        Await connection from QMP Monitor and perform capabilities 
> negotiation.
> +
> +        :param timeout:
> +            timeout in seconds (nonnegative float number, or None).
> +            If None, there is no timeout, and this may block forever.
> +
> +        :return: QMP greeting dict
> +        :raise ConnectError: on connection errors
> +        """
>          self._aqmp.await_greeting = True
>          self._aqmp.negotiate = True
>  
> @@ -130,6 +172,12 @@ def accept(self, timeout: Optional[float] = 15.0) -> 
> QMPMessage:
>          return ret
>  
>      def cmd_obj(self, qmp_cmd: QMPMessage) -> QMPMessage:
> +        """
> +        Send a QMP command to the QMP Monitor.
> +
> +        :param qmp_cmd: QMP command to be sent as a Python dict
> +        :return: QMP response as a Python dict
> +        """
>          return dict(
>              self._sync(
>                  # pylint: disable=protected-access
> @@ -148,9 +196,9 @@ def cmd(self, name: str,
>          """
>          Build a QMP command and send it to the QMP Monitor.
>  
> -        @param name: command name (string)
> -        @param args: command arguments (dict)
> -        @param cmd_id: command id (dict, list, string or int)
> +        :param name: command name (string)
> +        :param args: command arguments (dict)
> +        :param cmd_id: command id (dict, list, string or int)
>          """
>          qmp_cmd: QMPMessage = {'execute': name}
>          if args:
> @@ -160,6 +208,9 @@ def cmd(self, name: str,
>          return self.cmd_obj(qmp_cmd)
>  
>      def command(self, cmd: str, **kwds: object) -> QMPReturnValue:
> +        """
> +        Build and send a QMP command to the monitor, report errors if any
> +        """
>          return self._sync(
>              self._aqmp.execute(cmd, kwds),
>              self._timeout
> @@ -167,6 +218,19 @@ def command(self, cmd: str, **kwds: object) -> 
> QMPReturnValue:
>  
>      def pull_event(self,
>                     wait: Union[bool, float] = False) -> Optional[QMPMessage]:
> +        """
> +        Pulls a single event.
> +
> +        :param wait:
> +            If False or 0, do not wait. Return None if no events ready.
> +            If True, wait forever until the next event.
> +            Otherwise, wait for the specified number of seconds.
> +
> +        :raise asyncio.TimeoutError:
> +            When a timeout is requested and the timeout period elapses.
> +
> +        :return: The first available QMP event, or None.
> +        """
>          if not wait:
>              # wait is False/0: "do not wait, do not except."
>              if self._aqmp.events.empty():
> @@ -187,6 +251,20 @@ def pull_event(self,
>          )
>  
>      def get_events(self, wait: Union[bool, float] = False) -> 
> List[QMPMessage]:
> +        """
> +        Get a list of QMP events and clear all pending events.
> +
> +        :param wait:
> +            If False or 0, do not wait. Return None if no events ready.
> +            If True, wait until we have at least one event.
> +            Otherwise, wait for up to the specified number of seconds for at
> +            least one event.
> +
> +        :raise asyncio.TimeoutError:
> +            When a timeout is requested and the timeout period elapses.
> +
> +        :return: A list of QMP events.
> +        """
>          events = [dict(x) for x in self._aqmp.events.clear()]
>          if events:
>              return events
> @@ -195,17 +273,33 @@ def get_events(self, wait: Union[bool, float] = False) 
> -> List[QMPMessage]:
>          return [event] if event is not None else []
>  
>      def clear_events(self) -> None:
> +        """Clear current list of pending events."""
>          self._aqmp.events.clear()
>  
>      def close(self) -> None:
> +        """Close the connection."""
>          self._sync(
>              self._aqmp.disconnect()
>          )
>  
>      def settimeout(self, timeout: Optional[float]) -> None:
> +        """
> +        Set the timeout for QMP RPC execution.
> +
> +        This timeout affects the `cmd`, `cmd_obj`, and `command` methods.
> +        The `accept`, `pull_event` and `get_event` methods have their
> +        own configurable timeouts.
> +
> +        :param timeout:
> +            timeout in seconds, or None.
> +            None will wait indefinitely.
> +        """
>          self._timeout = timeout
>  
>      def send_fd_scm(self, fd: int) -> None:
> +        """
> +        Send a file descriptor to the remote via SCM_RIGHTS.
> +        """
>          self._aqmp.send_fd_scm(fd)
>  
>      def __del__(self) -> None:

Reviewed-by: Beraldo Leal <bleal@redhat.com>

--
Beraldo