Re: [Django] #33699: Read ASGI request body from asyncio queue on-demand

[Django]

#33699: Read ASGI request body from asyncio queue on-demand
-------------------------------+------------------------------------
Reporter: Noxx | Owner: Noxx
Type: New feature | Status: assigned
Component: HTTP handling | Version: dev
Severity: Normal | Resolution:
Keywords: ASGI, async | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Changes (by Carlton Gibson):

* owner: nobody => Noxx
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/33699#comment:3>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#33699: Read ASGI request body from asyncio queue on-demand
-------------------------------+------------------------------------
Reporter: Noxx | Owner: Noxx
Type: New feature | Status: assigned
Component: HTTP handling | Version: dev
Severity: Normal | Resolution:
Keywords: ASGI, async | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Changes (by Carlton Gibson):

* needs_better_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/33699#comment:4>

[Django]

#33699: Read ASGI request body from asyncio queue on-demand
-------------------------------+------------------------------------

Reporter: Noxx | Owner: noneNote

Type: New feature | Status: assigned
Component: HTTP handling | Version: dev
Severity: Normal | Resolution:
Keywords: ASGI, async | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------

Changes (by noneNote):

* owner: Noxx => noneNote

--
Ticket URL: <https://code.djangoproject.com/ticket/33699#comment:5>

[Django]

#33699: Read ASGI request body from asyncio queue on-demand
-------------------------------+------------------------------------
Reporter: Noxx | Owner: noneNote

Type: New feature | Status: closed

Component: HTTP handling | Version: dev

Severity: Normal | Resolution: wontfix

Keywords: ASGI, async | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------

Changes (by Carlton Gibson):

* status: assigned => closed
* resolution: => wontfix

Comment:

Looking at this again, with an eye to #33738 — which is how we handle
`http.disconnect` event as they come in — it's not clear that we can
actually read the body file on demand (as nice as that might be in
theory).

The [https://asgi.readthedocs.io/en/latest/specs/www.html#request-receive-
event ASGI spec] is quite clear on the required behaviour:

* "...the body message serves ... as a trigger to actually run request
code (as you should not trigger on a connection opening alone)." (Elided
to bring out the force.)
* `more_body` (bool) – Signifies if there is additional content to come
(as part of a Request message). If `True`, the consuming application
should wait until it gets a chunk with this set to `False`. If `False`,
the request is complete and should be processed.

i.e. The application is not permitted to begin processing the request
until a `more_body: False` is received. You can't get that unless you read
the body.

On top of that, ref #33738, if you want to look for an `http.disconnect`,
however you might do that, you need to have got the body events, which
come in on the same queue, out of the way in order to do that. A handler
for a long-lived connection that wanted to check that the client hadn't
disconnected **before** processing an expensive operation on the submitted
body would be stuck.

Short of a PoC, and an unlikely change to the ASGI spec here, I think we
have to close this as wontfix.

--
Ticket URL: <https://code.djangoproject.com/ticket/33699#comment:6>

[Django]

#33699: Read ASGI request body from asyncio queue on-demand
-------------------------------+------------------------------------
Reporter: Noxx | Owner: noneNote
Type: New feature | Status: closed
Component: HTTP handling | Version: dev
Severity: Normal | Resolution: wontfix
Keywords: ASGI, async | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------

Comment (by Natalia Bidart):

#35592 was a duplicate.
--
Ticket URL: <https://code.djangoproject.com/ticket/33699#comment:7>

[Django]

#33699: Read ASGI request body from asyncio queue on-demand
-------------------------------+------------------------------------
Reporter: Noxx | Owner: noneNote
Type: New feature | Status: closed
Component: HTTP handling | Version: dev
Severity: Normal | Resolution: wontfix
Keywords: ASGI, async | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------

Comment (by Klaas van Schelven):

TBH I'm not wholly convinced on your reading of the ASGI standard.

1. The ellipsis in the quote from the first bullet read "the body message
serves as a way to stream large incoming HTTP bodies in chunks", and it is
not clear to me why there would be a "way to stream large incoming HTTP
bodies" on the ASGI http protocol level if ASGI applications are forbidden
to make use of this in a meaningful way.

2. Regarding the second quote, first part "should wait" could be read as
"should wait if the application needs the whole request body to make its
response"

3. Regarding the second quote, second part there is no need to read
"should be processed" as "should be processed no earlier than"

4. Your reading (correct or not) is in direct contradiction with the
second paragraph of the ASGI spec which reads in full "ASGI attempts to
preserve a simple application interface, **while providing an abstraction
that allows for data to be sent and received at any time**, and from
different application threads or processes." (emphasis mine)

You say the ASGI spec cannot be changed, but AFAIU the links between the
ASGI Team and the Django team are quite close, so this is not entirely
impossible.

As a final note: the 2 "other" big ASGI servers (not built by Andrew)
actually have a "Hello World" example in their right on the homepage / in
the quickstart that does in fact do no attempt to read the request (and
are thus in violation of the spec as understood by Carlton):

*
[uvicorn](https://github.com/encode/uvicorn/blob/c23cd24e6676ee0638d014d7000af1e6e0996bd6/README.md#L58)
*
[hypercorn](https://github.com/pgjones/hypercorn/blob/84d06b8cf47798d2df7722273341e720ec0ea102/docs/tutorials/quickstart.rst#L14)
--
Ticket URL: <https://code.djangoproject.com/ticket/33699#comment:8>

[Django]

#33699: Read ASGI request body from asyncio queue on-demand
-------------------------------+------------------------------------
Reporter: Noxx | Owner: noneNote
Type: New feature | Status: closed
Component: HTTP handling | Version: dev
Severity: Normal | Resolution: wontfix
Keywords: ASGI, async | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------

Comment (by Carlton Gibson):

See [https://github.com/django/asgiref/issues/66 this long discussion on
the asgiref repo] for more context.

Disconnect handling requires consuming the request body messages from the
receive queue.

If the ASGI could pass an open file descriptor for the request body to the
application, then in principle we could use that directly, rather than
needing to spool the body file (to memory or disk, depending on size).
That would loose the ability to have the server and application on
different servers, but that might be OK in many cases. (IDK)

That would need first the PoC, and then the spec change (or clarification,
if you prefer) to go with it.
--
Ticket URL: <https://code.djangoproject.com/ticket/33699#comment:9>

[Django]

#33699: Read ASGI request body from asyncio queue on-demand
-------------------------------+------------------------------------
Reporter: Noxx | Owner: noneNote
Type: New feature | Status: closed
Component: HTTP handling | Version: dev
Severity: Normal | Resolution: wontfix
Keywords: ASGI, async | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------

Comment (by Klaas van Schelven):

I have the feeling I'm missing something here but I really don't see it.

What does "disconnect handling" mean in this context? Disconnect handling
by the ASGI server or by the ASGI application?

From my perspective as an application developer, there's not much to
_handle_ I think? I just want to _trigger_ a disconnect. I fail to see why
this could not simply be done by letting go of the constraint that the
request body must be consumed before sending a response. i.e. for this
particular issue I don't think changing the type of `body` to a file
descriptor is actually required.

Responding to a quote in the mentioned thread:

> The concerns with consuming the body (potentially unnecessarily, but
then why send it?)

Because it's not always up to the client to determine what should be
consumed by the server. Enforcement of quota / max message sizes is my
personal use-case.

Another use-case (not concerning disconnects) is to reduce latency for big
uploads, because processing (and possibly even sending back the result)
can start earlier.

But my main point would be: given ASGI's stated goals, it's _very
surprising_ that it's not possible to implement (for http) a streaming
echo server.

Anyway, this is moving into "someone is wrong on the internet" territory
for me; I ran into this while evaluating the streaming capacities of
various server setups and have not tied myself to an ASGI setup yet, so
for me the take-away is simply that ASGI is not a good fit for my use-
case.

Background regarding the HTTP/1.1 spec, and the idea of send-before-full-
receive (outside of the ASGI context):
https://stackoverflow.com/questions/14250991/is-it-acceptable-for-a
-server-to-send-a-http-response-before-the-entire-request
--
Ticket URL: <https://code.djangoproject.com/ticket/33699#comment:10>

[Django]

#33699: Read ASGI request body from asyncio queue on-demand
-------------------------------+------------------------------------
Reporter: Noxx | Owner: noneNote
Type: New feature | Status: closed
Component: HTTP handling | Version: dev
Severity: Normal | Resolution: wontfix
Keywords: ASGI, async | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------

Comment (by Carlton Gibson):

> What does "disconnect handling" mean in this context?

It means responding appropriately to the the
[https://asgi.readthedocs.io/en/latest/specs/www.html#disconnect-receive-
event ASGI `http.disconnect` event] that is sent on the `receive` queue in
the case that the client disconnects, whilst (for example) a long running
task is in process. (Correctly responding to this is needed to allow
applications to cancel and clean up such long running tasks.)

> Anyway, this is moving into...

OK, I wish you good luck with your search. If you wanted to explore
options here then a Proof-of-Concept plus a discussion on the asgiref repo
would be the appropriate way forward.
--
Ticket URL: <https://code.djangoproject.com/ticket/33699#comment:11>