-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement support for async and iterators. #3
Conversation
This change makes the decorator work correctly for async functions, instead of always returning the same coroutine, which can only be awaited once and calls subsequent calls to fail. This also detects returned iterators, evaluates them to completion, and returns the result as a tuple. Prior to this change, an exhausted iterator would be returned.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you don't have time to write the generator proxy, that's something I could be interested in taking a stab at
self.called = False | ||
self.return_value: typing.Any = None | ||
self.func = func | ||
self.is_asyncgen = inspect.isasyncgenfunction(self.func) | ||
self.is_async = True if self.is_asyncgen else inspect.iscoroutinefunction(self.func) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe a comment about why you chose iscoroutinefunction vs something like isawaitable would be good
once/__init__.py
Outdated
if self.called: | ||
return self.return_value | ||
if self.is_asyncgen: | ||
self.return_value = tuple([i async for i in func(*args, **kwargs)]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a big caveat that needs detailing in the documentation, that the generator result gets converted to a tuple. I think the more correct version, that's also more work, would wrap the generator in a generator that extends the return_value list each time next is called. This is potentially surprising enough to call it a bug that warrants exclusion from this library until it's fully supported.
once/__init__.py
Outdated
if self.called: | ||
return self.return_value | ||
with self.lock: | ||
if self.called: | ||
return self.return_value | ||
self.return_value = func(*args, **kwargs) | ||
if isinstance(self.return_value, types.GeneratorType): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto, this is a deal breaker IMO
Also, better handle sync iterators to be API-compatible with the unwrapped function call by returning an iterator object instead of a tuple object.
@mattalbr - I would appreciate help with the generator proxy, especially in async. The sync one at least is API-compatible now, but I disabled async because it was not. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, I'll take a crack at the async generator in a follow up
This change makes the decorator work correctly for async functions, instead of always returning the same coroutine, which can only be awaited once and calls subsequent calls to fail.
This also detects returned iterators, evaluates them to completion, and returns the result as a tuple. Prior to this change, an exhausted iterator would be returned.