On Thu, Dec 16, 2021 at 7:39 AM Beraldo Leal <bl...@redhat.com> wrote:
> 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 <js...@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. ;) > > Yeah, IMO it's 100% legitimate to define cleanup actions using catch-all except statements as long as you re-raise. There's no other construct that handles this case, AFAIK. try except <- on error else <- on NOT error finally <- always Sometimes you just wanna undo stuff only on error. Maybe a higher layer will "handle" that exception, maybe it'll crash the program. We don't know here, we just know we need to undo some stuff. > > > > 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 <bl...@redhat.com> > > ty :)
