diff mbox series

[03/15] python/aqmp: Return cleared events from EventListener.clear()

Message ID 20210917054047.2042843-4-jsnow@redhat.com (mailing list archive)
State New, archived
Headers show
Series Switch iotests to using Async QMP | expand

Commit Message

John Snow Sept. 17, 2021, 5:40 a.m. UTC
This serves two purposes:

(1) It is now possible to discern whether or not clear() removed any
event(s) from the queue with absolute certainty, and

(2) It is now very easy to get a List of all pending events in one
chunk, which is useful for the sync bridge.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/qemu/aqmp/events.py | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Comments

Hanna Czenczek Sept. 17, 2021, 12:36 p.m. UTC | #1
On 17.09.21 07:40, John Snow wrote:
> This serves two purposes:
>
> (1) It is now possible to discern whether or not clear() removed any
> event(s) from the queue with absolute certainty, and
>
> (2) It is now very easy to get a List of all pending events in one
> chunk, which is useful for the sync bridge.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>   python/qemu/aqmp/events.py | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)

Not sure if `clear` is an ideal name then, but `drain` sounds like 
something that would block, and `drop` is really much different from 
`clear`.  Also, doesn’t matter, having Collection.delete return the 
deleted element is a common thing in any language’s standard library, so 
why not have `clear` do the same.

Reviewed-by: Hanna Reitz <hreitz@redhat.com>
John Snow Sept. 17, 2021, 5:19 p.m. UTC | #2
On Fri, Sep 17, 2021 at 8:36 AM Hanna Reitz <hreitz@redhat.com> wrote:

> On 17.09.21 07:40, John Snow wrote:
> > This serves two purposes:
> >
> > (1) It is now possible to discern whether or not clear() removed any
> > event(s) from the queue with absolute certainty, and
> >
> > (2) It is now very easy to get a List of all pending events in one
> > chunk, which is useful for the sync bridge.
> >
> > Signed-off-by: John Snow <jsnow@redhat.com>
> > ---
> >   python/qemu/aqmp/events.py | 9 +++++++--
> >   1 file changed, 7 insertions(+), 2 deletions(-)
>
> Not sure if `clear` is an ideal name then, but `drain` sounds like
> something that would block, and `drop` is really much different from
> `clear`.  Also, doesn’t matter, having Collection.delete return the
> deleted element is a common thing in any language’s standard library, so
> why not have `clear` do the same.
>
>
It isn't too late to change the name, but it sounds like you don't
necessarily prefer any of those others over what's there now.


> Reviewed-by: Hanna Reitz <hreitz@redhat.com>
>
>
Thanks!
Hanna Czenczek Oct. 4, 2021, 9:03 a.m. UTC | #3
On 17.09.21 19:19, John Snow wrote:
>
>
> On Fri, Sep 17, 2021 at 8:36 AM Hanna Reitz <hreitz@redhat.com 
> <mailto:hreitz@redhat.com>> wrote:
>
>     On 17.09.21 07:40, John Snow wrote:
>     > This serves two purposes:
>     >
>     > (1) It is now possible to discern whether or not clear() removed any
>     > event(s) from the queue with absolute certainty, and
>     >
>     > (2) It is now very easy to get a List of all pending events in one
>     > chunk, which is useful for the sync bridge.
>     >
>     > Signed-off-by: John Snow <jsnow@redhat.com
>     <mailto:jsnow@redhat.com>>
>     > ---
>     >   python/qemu/aqmp/events.py | 9 +++++++--
>     >   1 file changed, 7 insertions(+), 2 deletions(-)
>
>     Not sure if `clear` is an ideal name then, but `drain` sounds like
>     something that would block, and `drop` is really much different from
>     `clear`.  Also, doesn’t matter, having Collection.delete return the
>     deleted element is a common thing in any language’s standard
>     library, so
>     why not have `clear` do the same.
>
>
> It isn't too late to change the name, but it sounds like you don't 
> necessarily prefer any of those others over what's there now.

Oh, no, I was just thinking aloud.

Hanna
diff mbox series

Patch

diff --git a/python/qemu/aqmp/events.py b/python/qemu/aqmp/events.py
index 271899f6b8..5f7150c78d 100644
--- a/python/qemu/aqmp/events.py
+++ b/python/qemu/aqmp/events.py
@@ -562,7 +562,7 @@  def empty(self) -> bool:
         """
         return self._queue.empty()
 
-    def clear(self) -> None:
+    def clear(self) -> List[Message]:
         """
         Clear this listener of all pending events.
 
@@ -570,17 +570,22 @@  def clear(self) -> None:
         pending FIFO queue synchronously. It can be also be used to
         manually clear any pending events, if desired.
 
+        :return: The cleared events, if any.
+
         .. warning::
             Take care when discarding events. Cleared events will be
             silently tossed on the floor. All events that were ever
             accepted by this listener are visible in `history()`.
         """
+        events = []
         while True:
             try:
-                self._queue.get_nowait()
+                events.append(self._queue.get_nowait())
             except asyncio.QueueEmpty:
                 break
 
+        return events
+
     def __aiter__(self) -> AsyncIterator[Message]:
         return self