diff mbox series

[06/10] python/aqmp: copy qmp docstrings to qemu.aqmp.legacy

Message ID 20220321210847.914787-7-jsnow@redhat.com (mailing list archive)
State New, archived
Headers show
Series Python: Remove synchronous QMP library | expand

Commit Message

John Snow March 21, 2022, 9:08 p.m. UTC
Copy the docstrings out of qemu.qmp, adjusting them as necessary to
more accurately reflect the current state of this class.

(Licensing: This is copying and modifying GPLv2-only licensed docstrings
into a GPLv2-only file.)

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Beraldo Leal <bleal@redhat.com>
---
 python/qemu/aqmp/legacy.py | 102 ++++++++++++++++++++++++++++++++++---
 1 file changed, 94 insertions(+), 8 deletions(-)

Comments

Hanna Czenczek March 23, 2022, 6:24 p.m. UTC | #1
On 21.03.22 22:08, John Snow wrote:
> Copy the docstrings out of qemu.qmp, adjusting them as necessary to
> more accurately reflect the current state of this class.
>
> (Licensing: This is copying and modifying GPLv2-only licensed docstrings
> into a GPLv2-only file.)
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Reviewed-by: Beraldo Leal <bleal@redhat.com>
> ---
>   python/qemu/aqmp/legacy.py | 102 ++++++++++++++++++++++++++++++++++---
>   1 file changed, 94 insertions(+), 8 deletions(-)
>
> diff --git a/python/qemu/aqmp/legacy.py b/python/qemu/aqmp/legacy.py
> index 10c7c99c4f..20ffdd8956 100644
> --- a/python/qemu/aqmp/legacy.py
> +++ b/python/qemu/aqmp/legacy.py

[...]

> @@ -60,6 +63,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')

Can’t help but notice that this is (technically) just wrong, because it 
isn’t ignored.  Are you afraid people might not be sufficiently 
dissuaded by the “deprecated” keyword?  (I mean, if that were the case, 
I’d suggest you write “Deprecated, because setting this to true might 
make your computer explode” instead.)

Hanna

> +    :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,
>                    nickname: Optional[str] = None):
John Snow March 23, 2022, 7:36 p.m. UTC | #2
On Wed, Mar 23, 2022 at 2:24 PM Hanna Reitz <hreitz@redhat.com> wrote:
>
> On 21.03.22 22:08, John Snow wrote:
> > Copy the docstrings out of qemu.qmp, adjusting them as necessary to
> > more accurately reflect the current state of this class.
> >
> > (Licensing: This is copying and modifying GPLv2-only licensed docstrings
> > into a GPLv2-only file.)
> >
> > Signed-off-by: John Snow <jsnow@redhat.com>
> > Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> > Reviewed-by: Beraldo Leal <bleal@redhat.com>
> > ---
> >   python/qemu/aqmp/legacy.py | 102 ++++++++++++++++++++++++++++++++++---
> >   1 file changed, 94 insertions(+), 8 deletions(-)
> >
> > diff --git a/python/qemu/aqmp/legacy.py b/python/qemu/aqmp/legacy.py
> > index 10c7c99c4f..20ffdd8956 100644
> > --- a/python/qemu/aqmp/legacy.py
> > +++ b/python/qemu/aqmp/legacy.py
>
> [...]
>
> > @@ -60,6 +63,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')
>
> Can’t help but notice that this is (technically) just wrong, because it
> isn’t ignored.  Are you afraid people might not be sufficiently
> dissuaded by the “deprecated” keyword?  (I mean, if that were the case,
> I’d suggest you write “Deprecated, because setting this to true might
> make your computer explode” instead.)
>

Oops, this is outdated. I *did* drop this parameter. once. And then I
re-added it.

Thanks for noticing.

--js
diff mbox series

Patch

diff --git a/python/qemu/aqmp/legacy.py b/python/qemu/aqmp/legacy.py
index 10c7c99c4f..20ffdd8956 100644
--- a/python/qemu/aqmp/legacy.py
+++ b/python/qemu/aqmp/legacy.py
@@ -1,7 +1,13 @@ 
 """
-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. It is provided here for
+compatibility with scripts inside the QEMU source tree that expect the
+old interface.
 """
 
 #
@@ -50,9 +56,6 @@ 
 # {} is the QMPReturnValue.
 
 
-# pylint: disable=missing-docstring
-
-
 class QMPBadPortError(QMPError):
     """
     Unable to parse socket address: Port was non-numerical.
@@ -60,6 +63,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,
                  nickname: Optional[str] = None):
@@ -121,6 +139,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
 
@@ -130,6 +154,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
 
@@ -140,6 +174,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
@@ -158,9 +198,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:
@@ -170,6 +210,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
@@ -177,6 +220,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():
@@ -197,6 +253,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
@@ -205,17 +275,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: