Re: [PATCH v2 02/25] python/aqmp: handle asyncio.TimeoutError on execute()

[Beraldo Leal <bleal@redhat.com>]

On Wed, Dec 15, 2021 at 02:39:16PM -0500, John Snow wrote:
> This exception can be injected into any await statement. If we are
> canceled via timeout, we want to clear the pending execution record on
> our way out.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  python/qemu/aqmp/qmp_client.py | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/python/qemu/aqmp/qmp_client.py b/python/qemu/aqmp/qmp_client.py
> index 8105e29fa8..6a985ffe30 100644
> --- a/python/qemu/aqmp/qmp_client.py
> +++ b/python/qemu/aqmp/qmp_client.py
> @@ -435,7 +435,11 @@ async def _issue(self, msg: Message) -> Union[None, str]:
>              msg_id = msg['id']
>  
>          self._pending[msg_id] = asyncio.Queue(maxsize=1)
> -        await self._outgoing.put(msg)
> +        try:
> +            await self._outgoing.put(msg)
> +        except:
> +            del self._pending[msg_id]
> +            raise

At first glance both, except and raise are not good practices, but I
think I got what you are doing here, just intercepting to delete the
pending message and raising it again. So maybe it will be safe to close
the eyes here. ;)

>  
>          return msg_id
>  
> @@ -452,9 +456,9 @@ async def _reply(self, msg_id: Union[str, None]) -> Message:
>              was lost, or some other problem.
>          """
>          queue = self._pending[msg_id]
> -        result = await queue.get()
>  
>          try:
> +            result = await queue.get()
>              if isinstance(result, ExecInterruptedError):
>                  raise result
>              return result

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

--
Beraldo