Skip to content

Commit

Permalink
Switch async_to_sync type error to a warning
Browse files Browse the repository at this point in the history
Detecting if a function is async in Python has a lot of false
negatives, so this is a lot safer while async code we call all makes
sure it has the coroutinefunction flag set.
  • Loading branch information
andrewgodwin committed Apr 6, 2021
1 parent 07bffa4 commit d0ce78b
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 7 deletions.
5 changes: 4 additions & 1 deletion asgiref/sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import os
import sys
import threading
import warnings
import weakref
from concurrent.futures import Future, ThreadPoolExecutor
from typing import Any, Callable, Dict, Optional, Union
Expand Down Expand Up @@ -118,7 +119,9 @@ class AsyncToSync:

def __init__(self, awaitable, force_new_loop=False):
if not callable(awaitable) or not _iscoroutinefunction_or_partial(awaitable):
raise TypeError("async_to_sync can only be applied to async functions.")
# Python does not have very reliable detection of async functions
# (lots of false negatives) so this is just a warning.
warnings.warn("async_to_sync was passed a non-async-marked callable")
self.awaitable = awaitable
try:
self.__self__ = self.awaitable.__self__
Expand Down
12 changes: 6 additions & 6 deletions tests/test_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -203,26 +203,26 @@ def test_async_to_sync_fail_non_function():
"""
async_to_sync raises a TypeError when applied to a non-function.
"""
with pytest.raises(TypeError) as excinfo:
with pytest.warns(UserWarning) as warnings:
async_to_sync(1)

assert excinfo.value.args == (
"async_to_sync can only be applied to async functions.",
assert warnings[0].message.args == (
"async_to_sync was passed a non-async-marked callable",
)


def test_async_to_sync_fail_sync():
"""
async_to_sync raises a TypeError when applied to a sync function.
"""
with pytest.raises(TypeError) as excinfo:
with pytest.warns(UserWarning) as warnings:

@async_to_sync
def test_function(self):
pass

assert excinfo.value.args == (
"async_to_sync can only be applied to async functions.",
assert warnings[0].message.args == (
"async_to_sync was passed a non-async-marked callable",
)


Expand Down

5 comments on commit d0ce78b

@adamchainz
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was there any particular case that came up after 3.3.3 that lead to this downgrade? Since it looked to me like #252 fixed the known problems.

@andrewgodwin
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, Daphne was using a lambda as an awaitable for send rather than a partial, so anyone running it still got the error. A patch is in the works to make it a partial and then maybe we can reintroduce the error.

@carltongibson
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, it's @adamchainz's patch. 😀

(I am a bit concerned they'll be other cases come up ... — but that's not based on much beyond a general aversion to changing anything ever. 🙂)

@andrewgodwin
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I plan to leave it as a warning for a bit longer, I think. Just to make sure it's not popping up everywhere.

@carltongibson
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 I think that's wise. Super.

Please sign in to comment.