django
django copied to clipboard
Fixed #33699 -- Made ASGI request read body on-demand.
Prior to this change, the request has been written into a spooled temporary file as the HTTPRequest class depends on a byte/io-like stream to process and parse its content as a whole. As the ASGI request is given in seperate chunks, those chunks has been concatenated first using this file.
However, doing so resulted in an increasing latency on the server side, as the file needed to be always written and re-read to be parsed afterwards. This was especially bad for big file uploads resulting in (Gateway) Timeouts if the Django server was running behind a reverse-proxy.
This change fixes this issue by wrapping the ASGI request and providing a byte/io-like stream interface to it. Doing so, reading the request's content is delayed until it needs to be parsed and than directly delivered into the upload-handlers, resulting in a reduced latency.
This PR references the discussion on https://groups.google.com/g/django-developers/c/fu6ZSmu-YJE.
Hello @Flauschbaellchen! Thank you for your contribution 💪
As it's your first contribution be sure to check out the patch review checklist.
If you're fixing a ticket from Trac make sure to set the "Has patch" flag and include a link to this PR in the ticket!
If you have any design or process questions then you can ask in the Django forum.
Welcome aboard ⛵️!
Hi @Flauschbaellchen — thanks for this! First thing is to address the CI failures so we've got a clean slate — Looks like asgi.tests.ASGITest.test_disconnect
(plus the lint errors).
@carltongibson Can I ask you to take a look into the asgi.tests.ASGITest.test_disconnect
test and help me out to fix this?
As the reading is now delayed until the HTTPRequest
is parsing the body, the ASGIStream
and its receive()
is not called currently within the test - and thus no TimeoutError
is raised.
I think the AsyncRequestFactory
/ApplicationCommunicator
needs to be adjusted, but I cannot see what I'd need to change.
@Flauschbaellchen — OK, let me take a look. (🤹 — Might be next week.)
Hey @Flauschbaellchen — I'm picking this up now.
Can I ask, could you put together a minimal app showing the upload/download example you raised in the discussion? You're creating a 1GB file; uploading; waiting for what? The same file sent back? etc. — If you could put together the minimal Django app for that, we can run it with gunicorn/uvicorn/Daphne/etc. and see the differences. (There's a wider need to set up such benchmarks, so this would tie into that too.)
Thanks
Closing as per ticket.