This will allow both seamless hosting of Django on ASGI servers, and will
also significantly improve performance for sites that have slow file
uploads as a blocking operation, as this will run asynchronously rather
than holding open a thread.
--
Ticket URL: <https://code.djangoproject.com/ticket/30451>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Comment (by Andrew Godwin):
The pull request for this is https://github.com/django/django/pull/11209
--
Ticket URL: <https://code.djangoproject.com/ticket/30451#comment:1>
* cc: Petter Strandmark (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/30451#comment:2>
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/30451#comment:3>
Comment (by Carlton Gibson):
> ...and will also significantly improve performance for sites that have
slow file uploads as a blocking operation, as this will run asynchronously
rather than holding open a thread.
From the conversation on the PR, it looks as if file uploads aren't
**yet** enabled:
> Florian: This probably cannot use the upload handlers for similar
reasons?
> Andrew: I haven't looked into that yet, but probably not. File input in
general has been a pain in Channels and I expect the same here.
> https://github.com/django/django/pull/11209#pullrequestreview-232567459
Is that right? If so do we need to pull this aspect into a separate
ticket?
--
Ticket URL: <https://code.djangoproject.com/ticket/30451#comment:4>
Comment (by Ran Benita):
It looks like the patch is going to be merged. I feel like I should at
least point out some downsides before this step is taken. (Note, I am not
a Django dev, just a user).
The patch adds an unconditional asyncio import. It is quite heavy for
something most users won't use at least initially. On my aging laptop:
- Import time: 50ms
- Memory usage: ~5MB
Command for import time: `python3.7 -X importtime -c 'import asyncio' |&
tail -1`
Script for memory usage:
{{{
#!python
from resource import getrusage, RUSAGE_SELF
before = getrusage(RUSAGE_SELF)
import asyncio
after = getrusage(RUSAGE_SELF)
print(f'{after.ru_maxrss - before.ru_maxrss}kb')
}}}
The patch adds 2 dependencies, one direct `asgiref` and one transitive
`async_timeout`.
The patch implements the ASGI specification, but that specification has
not been adopted widely yet and has not been proposed for official status
(that I know of) like WSGI has.
ASGI depends on asyncio, but there are other alternatives worth
considering (like trio and curio). And of course there is gevent which has
been used successfully by many companies (including mine) for async Djagno
for some time.
The patch on its own will run all views in a thread pool
(asgiref.sync_to_async -> loop.run_in_executor -> default
ThreadPoolExecutor). The default number of workers used is the number of
cores * 5, so for example an 8 core will use 40 workers. If postgresql is
used with persistent connections, this is a hazard since postgresql
doesn't like this many connections. So users will need to use a connection
pool, or disable persistent connections.
In my opinion, it would be hasty be merge ASGI support before the overall
plan is decided. If a DEP is later proposed, and rejected (I hope not!),
will it still make sense to have merged this?
--
Ticket URL: <https://code.djangoproject.com/ticket/30451#comment:5>
Comment (by Andrew Godwin):
Regarding ASGI not being "official": Python core has made it reasonably
clear to me that they don't want to have a PEP for ASGI, and giving one to
WSGI was a bad idea in the first place. Thus, adoption is down to us just
doing it. Flask have also said they are going to attempt to support it, as
have Twisted.
The threads observation is an astute one, and I think worth calling out in
the deployment documentation explicitly.
The async_timeout dependency is only for the async test harness in
asgiref, and thus should never be imported under normal conditions.
The reason I have done this separately from the DEP is that it's not, in
my eyes, strictly a new "feature" - instead, it is adding compatibility
for a server-application protocol, and some last-resort async safety. Even
if we didn't implement ASGI, I would still argue that we definitely need
to make the ORM understand and protect itself from coroutine-based
connection corruption, and that would involve importing asyncio (and
probably asgiref for its helpful features in this area) anyway!
--
Ticket URL: <https://code.djangoproject.com/ticket/30451#comment:6>
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/30451#comment:7>
Comment (by Andrew Godwin <andrew@…>):
In [changeset:"415e899dc46c2f8d667ff11d3e54eff759eaded4" 415e899d]:
{{{
#!CommitTicketReference repository=""
revision="415e899dc46c2f8d667ff11d3e54eff759eaded4"
Refs #30451 -- Added HttpRequest._set_content_type_params() hook.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30451#comment:8>
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"7f19e3713598a37b0809b5434114140170d12e34" 7f19e37]:
{{{
#!CommitTicketReference repository=""
revision="7f19e3713598a37b0809b5434114140170d12e34"
Refs #30451 -- Added more tests for ASGIRequest and ASGIHandler.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30451#comment:10>
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"a415ce70bef6d91036b00dd2c8544aed7aeeaaed" a415ce70]:
{{{
#!CommitTicketReference repository=""
revision="a415ce70bef6d91036b00dd2c8544aed7aeeaaed"
Fixed #30451 -- Added ASGI handler and coroutine-safety.
This adds an ASGI handler, asgi.py file for the default project layout,
a few async utilities and adds async-safety to many parts of Django.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30451#comment:9>
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"19895e897c9aa620de7db4dfd2fb19774075b755" 19895e89]:
{{{
#!CommitTicketReference repository=""
revision="19895e897c9aa620de7db4dfd2fb19774075b755"
Refs #30451 -- Added asgiref to the tests requirements.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30451#comment:11>
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"45de0c299bfc8117b04bb47df61a676a5013c4ce" 45de0c29]:
{{{
#!CommitTicketReference repository=""
revision="45de0c299bfc8117b04bb47df61a676a5013c4ce"
[3.0.x] Refs #30451 -- Doc'd asynchronous support and async-safety.
Backport of 635a3f8e6e0303e8a87586ef6be4ab0cc01d7c0d from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30451#comment:12>
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"635a3f8e6e0303e8a87586ef6be4ab0cc01d7c0d" 635a3f8e]:
{{{
#!CommitTicketReference repository=""
revision="635a3f8e6e0303e8a87586ef6be4ab0cc01d7c0d"
Refs #30451 -- Doc'd asynchronous support and async-safety.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30451#comment:13>
Comment (by GitHub <noreply@…>):
In [changeset:"441103a04d1d167dc870eaaf90e3fba974f67c93" 441103a]:
{{{
#!CommitTicketReference repository=""
revision="441103a04d1d167dc870eaaf90e3fba974f67c93"
Refs #33173, Refs #30451 -- Fixed ResourceWarning from unclosed body files
in ASGI handler on Python 3.11+.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30451#comment:14>