-
-
Notifications
You must be signed in to change notification settings - Fork 948
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
Improve detection of async callables #1444
Changes from 5 commits
ff314dd
a1c0764
636a26f
f6935c3
7312456
4151df6
8438a9f
788f011
dd8b7ed
7c74668
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
import asyncio | ||
import functools | ||
import typing | ||
|
||
|
||
def iscoroutinefunction(obj: typing.Any) -> bool: | ||
while isinstance(obj, functools.partial): | ||
Kludex marked this conversation as resolved.
Show resolved
Hide resolved
|
||
obj = obj.func | ||
|
||
return asyncio.iscoroutinefunction(obj) or ( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi @Kludex, would it make sense to use I understand why we cannot just use the said function alone, since
I have looked at the code of Cheers,
This comment was marked as off-topic.
Sorry, something went wrong. |
||
callable(obj) and asyncio.iscoroutinefunction(obj.__call__) | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,79 @@ | ||
import functools | ||
|
||
from starlette._utils import iscoroutinefunction | ||
|
||
|
||
def test_async_func(): | ||
async def async_func(): | ||
... # pragma: no cover | ||
|
||
def func(): | ||
... # pragma: no cover | ||
|
||
assert iscoroutinefunction(async_func) | ||
assert not iscoroutinefunction(func) | ||
|
||
|
||
def test_async_partial(): | ||
Kludex marked this conversation as resolved.
Show resolved
Hide resolved
|
||
async def async_func(a, b): | ||
... # pragma: no cover | ||
|
||
def func(a, b): | ||
... # pragma: no cover | ||
|
||
partial = functools.partial(async_func, 1) | ||
assert iscoroutinefunction(partial) | ||
|
||
partial = functools.partial(func, 1) | ||
assert not iscoroutinefunction(partial) | ||
|
||
|
||
def test_async_method(): | ||
class Async: | ||
async def method(self): | ||
... # pragma: no cover | ||
|
||
class Sync: | ||
def method(self): | ||
... # pragma: no cover | ||
|
||
assert iscoroutinefunction(Async().method) | ||
assert not iscoroutinefunction(Sync().method) | ||
|
||
|
||
def test_async_object_call(): | ||
class Async: | ||
async def __call__(self): | ||
... # pragma: no cover | ||
|
||
class Sync: | ||
def __call__(self): | ||
... # pragma: no cover | ||
|
||
assert iscoroutinefunction(Async()) | ||
assert not iscoroutinefunction(Sync()) | ||
|
||
|
||
def test_async_partial_object_call(): | ||
class Async: | ||
async def __call__(self, a, b): | ||
... # pragma: no cover | ||
|
||
class Sync: | ||
def __call__(self, a, b): | ||
... # pragma: no cover | ||
|
||
partial = functools.partial(Async(), 1) | ||
assert iscoroutinefunction(partial) | ||
|
||
partial = functools.partial(Sync(), 1) | ||
assert not iscoroutinefunction(partial) | ||
|
||
|
||
def test_async_nested_partial(): | ||
async def async_func(a, b): | ||
... # pragma: no cover | ||
|
||
partial = functools.partial(async_func, b=2) | ||
nested_partial = functools.partial(partial, a=1) | ||
assert iscoroutinefunction(nested_partial) |
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.
Coming from reviewing #1644
This is a nit, but should we call this
is_async_callable
?AFAICT, having "something we can call and
await
" is all we care about when checking for apps or endpoints."Coroutine function" is a well-defined word in the Python glossary: a "function which returns a coroutine object", defined with "
async def
".https://docs.python.org/3/glossary.html#term-coroutine-function
Also, the current naming makes it look as a compatibility shim on
asyncio.iscoroutinefunction
which focuses on checking just the definition above^, whereas we do some more checks, such as looking for__call__
.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.
Sure. 👍 I'll adapt later today. Thanks for the review. 🙏