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.

Sign up for GitHub

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

Jump to bottom

Enhancement: Use Python stdlib couroutine loop.SendFile for sending files #1250

Open
Goldziher opened this issue Feb 28, 2023 · 2 comments
Open

Enhancement: Use Python stdlib couroutine loop.SendFile for sending files #1250

Goldziher opened this issue Feb 28, 2023 · 2 comments
Labels
Enhancement This is a new feature or request Help Wanted 🆘 This is good for people to work on

Comments

@Goldziher
Copy link
Contributor

Goldziher commented Feb 28, 2023 •
edited by polar-sh bot

We should use loop.sendfile (Introduced in Python 3.7) to send files and fallback to chunking. This is significantly more performant.

(edited by @JacobCoffee)

Fund with Polar
@Goldziher Goldziher added Enhancement This is a new feature or request Triage Required 🏥 This requires triage labels Feb 28, 2023
@Goldziher Goldziher self-assigned this Feb 28, 2023
@Goldziher Goldziher removed the Triage Required 🏥 This requires triage label Feb 28, 2023
@JacobCoffee JacobCoffee changed the title Enhancement: use SendFile We should use loop.sendfile to send files and fallback to chunking. This is significantly more performant. Enhancement: Swap to using Python stdlib couroutine loop.SendFile Feb 28, 2023
@JacobCoffee JacobCoffee changed the title Enhancement: Swap to using Python stdlib couroutine loop.SendFile Enhancement: Use Python stdlib couroutine loop.SendFile for sending files Feb 28, 2023
@Goldziher
Copy link
Contributor Author

Goldziher commented Mar 3, 2023 •
edited

Update- This appears to be more complex than I assumed. the sendfile method is not implemented on all loops at all. Furthermore, it requires access to the async transport, which is a low level abstraction defined by the asyncio implementation but not anyio.

PROS: dramatically faster file serving using the operating system.
CONS: a lot more complexity and only support on asyncio.

I think we have to do this - but it requires significant efforts to implement. An example of an implementation is in aiohttp, specifically here: https://github.com/aio-libs/aiohttp/blob/master/aiohttp/web_fileresponse.py#L83

Additional context: https://github.com/aio-libs/aiohttp/blob/master/aiohttp/web_fileresponse.py#L83

@Goldziher Goldziher removed their assignment Mar 3, 2023
@Goldziher Goldziher added the Help Wanted 🆘 This is good for people to work on label Mar 3, 2023
@peterschutt
Copy link
Contributor

specifically here: https://github.com/aio-libs/aiohttp/blob/master/aiohttp/web_fileresponse.py#L83

Additional context: https://github.com/aio-libs/aiohttp/blob/master/aiohttp/web_fileresponse.py#L83

I assume "Additional context:" should be a different link here?

@geeshta geeshta mentioned this issue Sep 29, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Enhancement This is a new feature or request Help Wanted 🆘 This is good for people to work on
Projects
Overview
Status: Ideas
Milestone
No milestone
Development

No branches or pull requests
2 participants
@peterschutt @Goldziher