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

[Hanna Reitz]

On 17.09.21 19:19, John Snow wrote:



On Fri, Sep 17, 2021 at 8:36 AM Hanna Reitz > 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 mailto:js...@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

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

[John Snow]

On Fri, Sep 17, 2021 at 8:36 AM Hanna Reitz  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 
> > ---
> >   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 
>
>
Thanks!

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

[Hanna Reitz]

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 
---
  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 

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

[John Snow]

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 
---
 python/qemu/aqmp/events.py | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

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
 
-- 
2.31.1