`AsgiWhiteNoise` and async `WhiteNoiseMiddleware` #359

Archmonger · 2022-02-07T11:36:17Z

Tasks

Create async whitenoise class
Use aiofiles for asynchronous file handling
Async Django Middleware
Compatibility with existing tests
Add new ASGI-specific tests
Update the docs to show ASGI configuration

Summary

I've created async variants for any methods/functions/classes that can benefit from aiofiles. For IO operations that currently do not have an async equivalent (such as the OS calls within WhiteNoise.find_file(...)), I've converted to async via asyncio.to_thread(...) (if available).

Django middleware is now sync and async compatible. Thus, the middleware will provide Django an async FileResponse when possible.

`WhiteNoise`

whitenoise.base.BaseWhiteNoise
- Similar to the old WhiteNoise class, but __call__ is now a stub.
whitenoise.wsgi.WhiteNoise
- Functionally identical to the old WhiteNoise class. Uses BaseWhiteNoise as a subclass to obtain most methods.
whitenoise.asgi.AsgiWhiteNoise
- Implements the same behavior as WhiteNoise, but using the ASGI interface (async/threading)

`Responders`

whitenoise.responders.StaticFile
- Added aget_response (async variant).
- Added aget_range_response (async variant).
- Added aget_range_not_satisfiable_response (async variant).
whitenoise.responders.AsyncSlicedFile
- Implements the same behavior as SlicedFile, but using the aiofiles context manager interface.
whitenoise.responders.SlicedFile
- Minor refactoring to keep visual similarity to AsyncSlicedFile.
whitenoise.responders.Redirect
- Added aget_response (async variant).

`Middleware`

whitenoise.middleware.WhiteNoiseMiddleware
- Rewritten to add async support.
whitenoise.middleware.AsyncWhiteNoiseFileResponse
- Variant that uses async responses when possible (Django 4.2+ ASGI).
- Used when Django's middleware system suggests using async
- Responses must be converted to sync on WSGI.
whitenoise.middleware.AsyncFileIterator
- Simple __aiter__ helper that allow Django file responses to use aiofiles.
- Required to not break WSGI compatibility by providing an isolated async environment for responses.
whitenoise.middleware.AsyncToSyncIterator
- Simple __aiter__ to __iter__ converter, since Django cannot use __aiter__ within WSGI.
- Far more production-ready than the built-in converter on Django 4.2+.

Design Considerations

Code Architecture

The vast majority of this PR is the addition of new async functions, rather than changing/removal of existing code. This is due to the infectious nature of asyncio.
We do have the option of allowing ASGI applications directly within WhiteNoise, rather than creating a whole separate AsgiWhiteNoise class. However, this would either rely on heuristics or an initialization arg. This seemed more error-prone than an explicit class.
Only a minimal subset of methods were converted to async. There is a performance benefit to converting every method in BaseWhiteNoise to async, since event loops perform best with as much yielding via await as possible. However, that's a really high maintenance burden that we shouldn't commit to in this PR.
I tried to retain as much visual similarity between sync/async variants as possible to improve maintainability. Most async methods match up line-to-line with their sync counterparts. Minor changes were sometimes made to sync code to increase visual similarity.
Tests were only written to accommodate for the new async-specific bits. Since all non-file methods are still sync, there isn't a particular need to write tests that hit the same code.

Django Integration

Our middleware will automatically use sync or async responses depending on Django's context-aware sync/async auto-selection. Django tries to reduce the amount of sync/async context switches. If we are preceded by a bunch of sync middleware then Django will tell us to use sync.
It is necessary for async middleware to be able to convert responses to sync. This is because WSGI supports async middleware but not async file responses. Our conversion is done within a thread, which is far more production-ready than Django core's built-in method of consuming all iterators.
aiofiles is now a mandatory requirement due to Django middleware being async compatible. The alternative would have been a completely separate middleware class for ASGI, but that would negate a lot of the benefits of Django's middleware's context-aware sync/async auto-selection. Additionally, would have been a significant amount of extra code that would not have been worth the maintenance burden.
By default, we send ASGI responses containing 8192 byte chunks of file data (same as wsgiref.FileWrapper).

Feature Compatibility

I did not implement Zero Copy Send support due to no ASGI webservers having compatibility for this yet. No point in adding a feature that can't be used.
We use asgiref.compatibility.guarantee_single_callable to retain compatibility for both ASGI v2 and ASGI v3.

Test Environment: `AsgiWhiteNoise`

pip install git+https://github.com/Archmonger/whitenoise@asgi-compat

# example.py
from whitenoise import AsgiWhiteNoise

async def asgi_app(scope, receive, send):
    await send({
        'type': 'http.response.start',
        'status': 200,
        'headers': [(b'content-type', b'text/plain')],
    })
    await send({
        'type': 'http.response.body',
        'body': b'Hello, world!',
    })

# Note: This also works with legacy ASGI v2 applications
app = AsgiWhiteNoise(asgi_app, root='static', prefix='static')

# bash
uvicorn example:app

Test Environment: `WhiteNoiseMiddleware`

pip install git+https://github.com/Archmonger/whitenoise@asgi-compat

The existing middleware has been replaced with an async compatible variant. Therefore, follow the normal documentation for installing WhiteNoiseMiddleware.

Issues

fix #251
fix #179
close #261
related #181

for more information, see https://pre-commit.ci

…into asgi-compat

.gitignore

setup.cfg

tests/test_asgi.py

whitenoise/asgi.py

adamchainz · 2022-02-07T23:11:36Z

whitenoise/asgi.py

+            content_length = int(dict(response.headers)["Content-Length"])
+            for block in read_file(response.file, content_length, block_size):
+                await send(
+                    {"type": "http.response.body", "body": block, "more_body": True}
+                )


So ASGI has a "zero copy send" extension: https://asgi.readthedocs.io/en/latest/extensions.html#zero-copy-send

This is much more efficient than chunking and recombining the file in Python

That said, it looks like support isn't there yet:

daphne - none

hypercorn - none

uvicorn - pending Support zerocopy send encode/uvicorn#1210

So it might be worth creating a follow-up issue for this

there's also a PR for hypercorn, jfyk: https://gitlab.com/pgjones/hypercorn/-/merge_requests/62

in any case, none of the ASGI servers supports it yet 👍

Has anything changed in the "zero copy send" landscape?

Has anything changed in the "zero copy send" landscape?

I've created a fork of hypercorn to add all extension support here, you can have a try, currently it's under development and should work with all asgi extension listed in asgi spec.

Even when zero copy send gets merged into most ASGI webservers, there's still the question of Windows compatibility.

Ref: https://discuss.python.org/t/support-for-os-sendfile-for-windows/25020

tests/test_asgi.py

whitenoise/asgi.py

This reverts commit 8590e82.

for more information, see https://pre-commit.ci

This reverts commit f4931e3.

This reverts commit 8a0930e.

adamchainz · 2022-02-10T09:25:45Z

Set up pre-commit locally - install as per https://pre-commit.com/ , then pre-commit install in the repo.

setup.cfg

Co-authored-by: Adam Johnson <me@adamj.eu>

Archmonger · 2023-08-06T10:09:10Z

@evansd Given that I've had to read every file within this repo for the sake of this PR, I wouldn't mind helping out as a maintainer. That is, if you're looking for additional support.

We are planning on integrating AsgiWhiteNoise within ReactPy, so it's in my best interest that this repo stays maintained.

daniel-brenot · 2023-08-24T19:35:31Z

Bumping just to ask, is there anything left on this PR or can it be merged? It would really help us.

evansd · 2023-08-24T20:08:15Z

Hi, sorry this has gone unreviewed for so long. Just snowed under with other things at the moment. I am going to try to take a look at this as soon as I can. I think it's an important feature for Whitenoise. But it's a big change and I don't want to just merge it without proper consideration.

Archmonger · 2023-10-03T08:59:46Z

Since I don't want to get into a cat-and-mouse game of constantly re-merging with main, hopefully this PR can get reviewed soon.

robd003 · 2023-10-03T14:30:28Z

@evansd could you please review this?

robd003 · 2023-10-07T20:32:44Z

Could @adamchainz approve this pull request if @evansd is too busy?

brianglass · 2023-10-17T22:02:03Z

@Archmonger, I'm running your branch in production in a Django app. I'll let you know if I have any issues, but so far so good.

robd003 · 2023-10-18T02:25:08Z

Can we get this merged in for the 6.7 release @evansd ?

brianglass · 2023-11-06T21:39:08Z

@Archmonger, when Whitenoise returns a 304 response for a file that is already cached on the client, it gives the following warning:

/Users/bglass/src/orthocal-python/ve/lib/python3.12/site-packages/django/http/response.py:517: Warning: StreamingHttpResponse must consume synchronous iterators in order to serve them asynchronously. Use an asynchronous iterator instead.

Is there something I should be doing to avoid this?

Archmonger · 2023-11-06T21:41:37Z

What Django version are you running?

brianglass · 2023-11-06T22:39:33Z

@Archmonger, I'm running Django 5.0b1. I just tried it with 4.2.7 with the same result. I though it might be django-cors-headers since I had the middleware before whitenoise, but I took it out and still got the warning.

Archmonger · 2023-11-06T23:56:45Z

Can you send me your middleware and installed apps? Also let's pull this discussion to the side so that we don't spam everyone with this conversation.

Discord: totoboto
Email: archiethemonger@gmail.com

helenap · 2023-11-08T06:38:33Z

Hey @evansd could you please review and merge this in? This branch has been working just fine for me for months!

daniel-brenot · 2024-01-09T21:20:24Z

Bumping this since it's been a couple more months. Can we get this merged and released? I'd love to get rid of the errors and sync penalty in my code.

robd003 · 2024-02-20T17:07:31Z

Hey @evansd any update on you looking at this?

jlost · 2024-03-06T20:11:44Z

Would like to see this!

duanhongyi · 2024-03-07T02:32:29Z

Can this PR be merged?

jordanorc · 2024-03-07T10:10:25Z

Any update on this PR?

Archmonger · 2024-03-07T10:15:18Z

@jordanorc This PR is complete, but has been stuck in an "awaiting review" stage due to the WhiteNoise maintainers largely not having time.

In order to avoid continuously re-merging with main, I've been waiting for a maintainer to review and/or approve.

I have offered to help as a maintainer but haven't heard anything back yet.

smathieson · 2024-03-07T22:28:26Z

Also would like to see this merged ASAP.

docs/asgi.rst

tests/test_asgi.py

Co-authored-by: James Ostrander <11338926+jlost@users.noreply.github.com>

charlesobrien · 2024-03-27T02:33:44Z

@Archmonger @adamchainz How can we get this one across the finish line? If it’s just resolving conflicts, would it be merged?

kmichel and others added 6 commits September 14, 2020 23:52

Add ASGI adapter

9a56981

add venv to gitignore

8590e82

Merge remote-tracking branch 'upstream/master' into asgi-compat

24c5319

add aiofile

391bd8e

[pre-commit.ci] auto fixes from pre-commit.com hooks

3e735ec

for more information, see https://pre-commit.ci

Merge branch 'asgi-compat' of https://github.com/Archmonger/whitenoise …

42fbe15

…into asgi-compat

adamchainz reviewed Feb 7, 2022

View reviewed changes

Archmonger and others added 15 commits February 7, 2022 22:18

Revert "add venv to gitignore"

aec9ac1

This reverts commit 8590e82.

add ASGI extra

cf4ee09

absolute imports

9616e78

use SimpleNamespace

35b9003

add zero-copy send todo

8eef8d3

replace empty equality with boolean operation

2f0f685

minor syntax or performance improvements

f4931e3

BaseWhiteNoise class

3fb5a38

Merge remote-tracking branch 'upstream/master' into asgi-compat

3bb4088

use f-strings

8a0930e

fix imports

e6fd03b

Merge remote-tracking branch 'upstream/master' into asgi-compat

8fc1682

[pre-commit.ci] auto fixes from pre-commit.com hooks

64fc5c7

for more information, see https://pre-commit.ci

Revert "minor syntax or performance improvements"

83fea65

This reverts commit f4931e3.

Revert "use f-strings"

58f3743

This reverts commit 8a0930e.

adamchainz reviewed Feb 10, 2022

View reviewed changes

setup.cfg Outdated Show resolved Hide resolved

Archmonger and others added 6 commits February 10, 2022 01:28

re-add missing import

f79aec7

Remove upper bound

e18df38

Co-authored-by: Adam Johnson <me@adamj.eu>

fix recceive call

45301b2

fix some of the tests

88ccf21

temporarily remove broken tests

998cf5e

fix py3.10 deprecation warning

d611e90

rename to AsyncSlicedFile

503e957

bcail mentioned this pull request Oct 19, 2023

Upgrade to Django 4.2 raises new warning #531

Open

robd003 mentioned this pull request Nov 7, 2023

Merge ASGIWhiteNoise #543

Open

Archmonger mentioned this pull request Nov 29, 2023

Re-evaluate "http.response.zerocopysend" django/asgiref#423

Open

jlost reviewed Mar 14, 2024

View reviewed changes

docs/asgi.rst Outdated Show resolved Hide resolved

docs/asgi.rst Outdated Show resolved Hide resolved

tests/test_asgi.py Outdated Show resolved Hide resolved

carltongibson mentioned this pull request Mar 15, 2024

TypeError: object HttpResponse can't be used in 'await' expression django/daphne#502

Closed

Archmonger and others added 3 commits March 22, 2024 23:25

Update tests/test_asgi.py

67f8dee

Co-authored-by: James Ostrander <11338926+jlost@users.noreply.github.com>

Update docs/asgi.rst

1f8c4b2

Co-authored-by: James Ostrander <11338926+jlost@users.noreply.github.com>

Update docs/asgi.rst

aa36dce

Co-authored-by: James Ostrander <11338926+jlost@users.noreply.github.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`AsgiWhiteNoise` and async `WhiteNoiseMiddleware` #359

`AsgiWhiteNoise` and async `WhiteNoiseMiddleware` #359

Archmonger commented Feb 7, 2022 •

edited

adamchainz Feb 7, 2022

Kludex May 4, 2022 •

edited

Archmonger Jun 21, 2023

Kludex Jun 22, 2023

synodriver Jul 18, 2023 •

edited

Archmonger Jul 26, 2023 •

edited

adamchainz commented Feb 10, 2022

Archmonger commented Aug 6, 2023 •

edited

daniel-brenot commented Aug 24, 2023

evansd commented Aug 24, 2023

Archmonger commented Oct 3, 2023

robd003 commented Oct 3, 2023

robd003 commented Oct 7, 2023

brianglass commented Oct 17, 2023

robd003 commented Oct 18, 2023

brianglass commented Nov 6, 2023

Archmonger commented Nov 6, 2023

brianglass commented Nov 6, 2023

Archmonger commented Nov 6, 2023

helenap commented Nov 8, 2023 •

edited

daniel-brenot commented Jan 9, 2024

robd003 commented Feb 20, 2024

jlost commented Mar 6, 2024

duanhongyi commented Mar 7, 2024

jordanorc commented Mar 7, 2024

Archmonger commented Mar 7, 2024

smathieson commented Mar 7, 2024

charlesobrien commented Mar 27, 2024

AsgiWhiteNoise and async WhiteNoiseMiddleware #359

Are you sure you want to change the base?

AsgiWhiteNoise and async WhiteNoiseMiddleware #359

Conversation

Archmonger commented Feb 7, 2022 • edited

Tasks

Summary

WhiteNoise

Responders

Middleware

Design Considerations

Code Architecture

Django Integration

Feature Compatibility

Test Environment: AsgiWhiteNoise

Test Environment: WhiteNoiseMiddleware

Issues

adamchainz Feb 7, 2022

Choose a reason for hiding this comment

Kludex May 4, 2022 • edited

Choose a reason for hiding this comment

Archmonger Jun 21, 2023

Choose a reason for hiding this comment

Kludex Jun 22, 2023

Choose a reason for hiding this comment

synodriver Jul 18, 2023 • edited

Choose a reason for hiding this comment

Archmonger Jul 26, 2023 • edited

Choose a reason for hiding this comment

adamchainz commented Feb 10, 2022

Archmonger commented Aug 6, 2023 • edited

daniel-brenot commented Aug 24, 2023

evansd commented Aug 24, 2023

Archmonger commented Oct 3, 2023

robd003 commented Oct 3, 2023

robd003 commented Oct 7, 2023

brianglass commented Oct 17, 2023

robd003 commented Oct 18, 2023

brianglass commented Nov 6, 2023

Archmonger commented Nov 6, 2023

brianglass commented Nov 6, 2023

Archmonger commented Nov 6, 2023

helenap commented Nov 8, 2023 • edited

daniel-brenot commented Jan 9, 2024

robd003 commented Feb 20, 2024

jlost commented Mar 6, 2024

duanhongyi commented Mar 7, 2024

jordanorc commented Mar 7, 2024

Archmonger commented Mar 7, 2024

smathieson commented Mar 7, 2024

charlesobrien commented Mar 27, 2024

`AsgiWhiteNoise` and async `WhiteNoiseMiddleware` #359

`AsgiWhiteNoise` and async `WhiteNoiseMiddleware` #359

Archmonger commented Feb 7, 2022 •

edited

`WhiteNoise`

`Responders`

`Middleware`

Test Environment: `AsgiWhiteNoise`

Test Environment: `WhiteNoiseMiddleware`

Kludex May 4, 2022 •

edited

synodriver Jul 18, 2023 •

edited

Archmonger Jul 26, 2023 •

edited

Archmonger commented Aug 6, 2023 •

edited

helenap commented Nov 8, 2023 •

edited