Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: "http.response.sendfile" extension. #184

Closed
abersheeran opened this issue Jul 30, 2020 · 12 comments · Fixed by #191
Closed

Proposal: "http.response.sendfile" extension. #184

abersheeran opened this issue Jul 30, 2020 · 12 comments · Fixed by #191

Comments

@abersheeran
Copy link
Contributor

Add the sendfile extension to allow ASGI programs to return response through sendfile.

In scope:

"scope": {
    ...
    "extensions": {
        "http.response.sendfile": {},
    },
}

Response:

await send({"type": "http.response.start", ...})
await send({"type": "http.response.sendfile", "file": file-obj})

Some related links:

https://docs.python.org/3.7/library/asyncio-eventloop.html#asyncio.loop.sendfile
encode/uvicorn#35

@andrewgodwin
Copy link
Member

Yes, I can see this being sensible, since having it as an extension means that things not on (a new enough) asyncio can just not provide it.

@abersheeran
Copy link
Contributor Author

Can I submit a PR to add this section to the ASGI document?

@andrewgodwin
Copy link
Member

Totally - I think it's best to iterate on a draft here, so we can make sure the specification is nailed down properly.

@abersheeran
Copy link
Contributor Author

Okay.

@davidism
Copy link

davidism commented Sep 1, 2020

It seems like this isn't (directly) related to X-SendFile in httpd/lighttpd and X-Accel-Redirect in Nginx. Those set headers pointing to a path (SendFile) or internal URI (Accel) that tell the HTTP server to do the sendfile call.

The extension should make it clear that these aren't the same. Or maybe it would be possible to accommodate them along with allowing the ASGI server to use loop.sendfile.

@andrewgodwin
Copy link
Member

Yeah, we need to make this very clear in terms of what it does and allows. Maybe a rename to make it clearer - something like http.response.direct-file-stream.

@abersheeran
Copy link
Contributor Author

All right. If the name isn't clear enough, we can consider using zero-copy. That's what this extension is all about. What do you think?

@andrewgodwin
Copy link
Member

I'd be OK with zero-copy, that seems reasonable, though maybe zero-copy-send to be a bit more precise?

@abersheeran
Copy link
Contributor Author

I agree. @andrewgodwin

@synodriver
Copy link

Hi, I'm trying to implement this extension in hypercorn, but hava some question about it. The document said file is an integer, while loop.sendfile accepts only file object. I'm using os.fdopen in this case, however, that just make more trouble. Here is my code that did not work. Can you give me some advice?

@abersheeran
Copy link
Contributor Author

abersheeran commented Oct 5, 2021

loop.sendfile just wraps os.sendfile.

The standard uses integer (file descriptor) to make it easier for servers written in C, Rust, and so on to handle this.

@abersheeran
Copy link
Contributor Author

@synodriver Maybe the PR encode/uvicorn#1210 can help you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants