On Thu, Aug 5, 2021 at 5:02 PM Eric Blake <ebl...@redhat.com> wrote: > On Tue, Aug 03, 2021 at 02:29:22PM -0400, John Snow wrote: > > This serves a few purposes: > > > > 1. Protect interfaces when it's not safe to call them (via @require) > > > > 2. Add an interface by which an async client can determine if the state > > has changed, for the purposes of connection management. > > > > Signed-off-by: John Snow <js...@redhat.com> > > --- > > python/qemu/aqmp/__init__.py | 6 +- > > python/qemu/aqmp/protocol.py | 159 ++++++++++++++++++++++++++++++++++- > > 2 files changed, 160 insertions(+), 5 deletions(-) > > > > > +++ b/python/qemu/aqmp/protocol.py > > + > > +F = TypeVar('F', bound=Callable[..., Any]) # pylint: > disable=invalid-name > > + > > + > > +# Don't Panic. > > +def require(required_state: Runstate) -> Callable[[F], F]: > > Technically, the definition of F is integral to the definition of > require, so I might have put the 'Don't Panic' note a bit sooner. And > it took me a while before I noticed... > > > + """ > > + Decorator: protect a method so it can only be run in a certain > `Runstate`. > > + > > + :param required_state: The `Runstate` required to invoke this > method. > > + :raise StateError: When the required `Runstate` is not met. > > + """ > > + def _decorator(func: F) -> F: > > + # _decorator is the decorator that is built by calling the > > + # require() decorator factory; e.g.: > > + # > > + # @require(Runstate.IDLE) def # foo(): ... > > Is that second # intentional? > > No, that one is a mistake. Comment reflow mishap.
> > + # will replace 'foo' with the result of '_decorator(foo)'. > > + > > + @wraps(func) > > + def _wrapper(proto: 'AsyncProtocol[Any]', > > + *args: Any, **kwargs: Any) -> Any: > > + # _wrapper is the function that gets executed prior to the > > + # decorated method. > > + > > + name = type(proto).__name__ > > + > > + if proto.runstate != required_state: > > + if proto.runstate == Runstate.CONNECTING: > > + emsg = f"{name} is currently connecting." > > + elif proto.runstate == Runstate.DISCONNECTING: > > + emsg = (f"{name} is disconnecting." > > + " Call disconnect() to return to IDLE > state.") > > + elif proto.runstate == Runstate.RUNNING: > > + emsg = f"{name} is already connected and running." > > + elif proto.runstate == Runstate.IDLE: > > + emsg = f"{name} is disconnected and idle." > > + else: > > + assert False > > + raise StateError(emsg, proto.runstate, required_state) > > + # No StateError, so call the wrapped method. > > + return func(proto, *args, **kwargs) > > + > > + # Return the decorated method; > > + # Transforming Func to Decorated[Func]. > > + return cast(F, _wrapper) > > + > > + # Return the decorator instance from the decorator factory. Phew! > > + return _decorator > > ...that it paired with this comment, explaining what looks like a > monstrosity, but in reality is a fairly typical and boilerplate > implementation of a function decorator (not that you write one every > day - the whole point of the decorator is to have a one-word > abstraction already implemented so you DON'T have to reimplement the > decoration yourself ;). > > > + > > + > > class AsyncProtocol(Generic[T]): > > """ > > AsyncProtocol implements a generic async message-based protocol. > > @@ -118,7 +202,24 @@ def __init__(self) -> None: > > #: exit. > > self._dc_task: Optional[asyncio.Future[None]] = None > > > > + self._runstate = Runstate.IDLE > > + self._runstate_changed: Optional[asyncio.Event] = None > > + > > + @property # @upper_half > > Is it intentional to leave the @upper_half decorator commented out? > > In this case yes, because you can't decorate a property in Python. So for the sake of leaving it greppable, especially when those decorators are just a "notation only" thing anyway, I figured I'd stuff it behind a comment. > But that's minor enough that I trust you to handle it as you see fit. > > Reviewed-by: Eric Blake <ebl...@redhat.com> > > Thanks! > -- > Eric Blake, Principal Software Engineer > Red Hat, Inc. +1-919-301-3266 > Virtualization: qemu.org | libvirt.org > >