Repository: django/channels
Pull request #1766 (2021-11-08 13:45:06)

Relax consumer_callable requirements with a test
See #1577 - I just added and formatted the suggested test as the first of two commits.

> According to [ASGI specification](https://asgi.readthedocs.io/en/latest/specs/main.html#applications), both receive and send should be an awaitable callable, where as [create_task()](https://docs.python.org/3/library/asyncio-eventloop.html#creating-futures-and-tasks), accept only coroutine.
> 
> [NGINX Unit](http://unit.nginx.org/) ASGI server implementation, provides receive and send callable which returns Future, which in turns is awaitable and can be used just as a Task.
> 
> Using ensure_future() instead of create_task() is a simple way to create Task for coroutines and use Future if it is already provided.

willstott101: Test run here: https://github.com/willstott101/channels/actions/runs/1435192934
willstott101: I've edited the original comment here with more context from #1577 

This is a very minor non-breaking code change that would unblock usage of channels under **NGINX Unit**

Would the maintainers here prefer a more simplified unit test for `await_many_dispatch`? It seems to me that a unit test for a utility function is not something that would catch this if it regresses again later, after say, refactoring and removing that utility function. That being said I'm very happy to replace that test if it helps?

willstott101: Worth noting that literally less than 12 lines below this change [ensure_future is already used in this same function for later instantiations of callables](https://github.com/django/channels/blob/7bc96b6554a995471574ab498bfc942f79d099ad/channels/utils.py#L52) meaning this utility function isn't even consistent. The changes here both make the behaviour of this function consistent - and more spec compliant. The test ensures spec compliance in this area for the `AsyncHttpConsumer` in an integration test style.

Failing test run (with existing code): https://github.com/willstott101/channels/actions/runs/1909911872
Passing test run (with patched code): https://github.com/willstott101/channels/actions/runs/1909905681

Patch:
diff --git a/channels/utils.py b/channels/utils.py
index 19c0d48..72cd9ca 100644
--- a/channels/utils.py
+++ b/channels/utils.py
@@ -34,10 +34,9 @@ async def await_many_dispatch(consumer_callables, dispatch):
     Given a set of consumer callables, awaits on them all and passes results
     from them to the dispatch awaitable as they come in.
     """
-    # Start them all off as tasks
-    loop = asyncio.get_event_loop()
+    # Call all callables, and ensure all return types are Futures
     tasks = [
-        loop.create_task(consumer_callable())
+        asyncio.ensure_future(consumer_callable())
         for consumer_callable in consumer_callables
     ]
     try:
diff --git a/docs/installation.rst b/docs/installation.rst
index 6e7ae5b..db48ab8 100644
--- a/docs/installation.rst
+++ b/docs/installation.rst
@@ -9,7 +9,7 @@ Channels is available on PyPI - to install it run:
 
 This will install Channels together with the Daphne ASGI application server. If
 you wish to use a different application server you can ``pip install channels``,
-without the optional ``daphe`` add-on.
+without the optional ``daphne`` add-on.
 
 Once that's done, you should add ``daphne`` to the beginning of your
 ``INSTALLED_APPS`` setting:


Test patch:
diff --git a/tests/test_generic_http.py b/tests/test_generic_http.py
index 99131e0..85ecdd0 100644
--- a/tests/test_generic_http.py
+++ b/tests/test_generic_http.py
@@ -1,3 +1,4 @@
+import asyncio
 import json
 import time
 
@@ -71,3 +72,56 @@ async def test_per_scope_consumers():
     assert second_response["status"] == 200
 
     assert response["body"] != second_response["body"]
+
+
+@pytest.mark.asyncio
+async def test_async_http_consumer_future():
+    """
+    Regression test for channels accepting only coroutines. The ASGI specification
+    states that the `receive` and `send` arguments to an ASGI application should be
+    "awaitable callable" objects. That includes non-coroutine functions that return
+    Futures.
+    """
+
+    class TestConsumer(AsyncHttpConsumer):
+        async def handle(self, body):
+            await self.send_response(
+                200,
+                b"42",
+                headers={b"Content-Type": b"text/plain"},
+            )
+
+    app = TestConsumer()
+
+    # Ensure the passed functions are specifically coroutines.
+    async def coroutine_app(scope, receive, send):
+        async def receive_coroutine():
+            return await asyncio.ensure_future(receive())
+
+        async def send_coroutine(*args, **kwargs):
+            return await asyncio.ensure_future(send(*args, **kwargs))
+
+        await app(scope, receive_coroutine, send_coroutine)
+
+    communicator = HttpCommunicator(coroutine_app, method="GET", path="/")
+    response = await communicator.get_response()
+    assert response["body"] == b"42"
+    assert response["status"] == 200
+    assert response["headers"] == [(b"Content-Type", b"text/plain")]
+
+    # Ensure the passed functions are "Awaitable Callables" and NOT coroutines.
+    async def awaitable_callable_app(scope, receive, send):
+        def receive_awaitable_callable():
+            return asyncio.ensure_future(receive())
+
+        def send_awaitable_callable(*args, **kwargs):
+            return asyncio.ensure_future(send(*args, **kwargs))
+
+        await app(scope, receive_awaitable_callable, send_awaitable_callable)
+
+    # Open a connection
+    communicator = HttpCommunicator(awaitable_callable_app, method="GET", path="/")
+    response = await communicator.get_response()
+    assert response["body"] == b"42"
+    assert response["status"] == 200
+    assert response["headers"] == [(b"Content-Type", b"text/plain")]