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

Fix Django "consume iterator" warning #13

Merged
merged 5 commits into from
Aug 27, 2024
Merged

Conversation

Archmonger
Copy link
Owner

@Archmonger Archmonger commented Aug 26, 2024

Description

There are some circumstances where Django >4.2 would show the following warning: Warning: StreamingHttpResponse must consume synchronous iterators in order to serve them asynchronously. Use an asynchronous iterator instead.

There are two culprits are that caused these warnings:

  • "304 file unchanged" file responses were being sent to Django with an empty tuple as the file content, which is technically not asynchronous.
  • Django's sync/async middleware auto selection prefers middleware to run in sync mode, even when it didn't make sense. This prevented ServeStatic from generating async file responses.

Changelog

  • Prevent responses with no body from creating a warning by providing Django a "placeholder" empty async iterable as the file content.
  • Force Django>=3.2 to try using async file responses, even if the middleware is not running in async mode.

Checklist

Please update this checklist as you complete each item:

  • Tests have been developed for bug fixes or new functionality.
  • The changelog has been updated, if necessary.
  • Documentation has been updated, if necessary.
  • GitHub Issues closed by this PR have been linked.

By submitting this pull request I agree that all contributions comply with this project's open source license(s).

@Archmonger Archmonger linked an issue Aug 26, 2024 that may be closed by this pull request
@Archmonger Archmonger marked this pull request as ready for review August 27, 2024 03:27
@Archmonger Archmonger merged commit 957f93c into main Aug 27, 2024
15 checks passed
@Archmonger Archmonger deleted the fix-django-iterator-warning branch August 27, 2024 04:06
@brianglass
Copy link

@Archmonger, I tried this out this morning. First, thank you for finding a solution for this issue and getting a fix out so promptly! Second, I do have a potential issue to report. I haven't dug into this yet, but I'm getting a new error from a middleware lower in the stack that looks like the following:

Traceback (most recent call last):
web-1  |   File "/usr/local/lib/python3.12/site-packages/django/core/handlers/exception.py", line 55, in inner
web-1  |     response = get_response(request)
web-1  |                ^^^^^^^^^^^^^^^^^^^^^
web-1  |   File "/usr/local/lib/python3.12/site-packages/google/cloud/logging_v2/handlers/middleware/request.py", line 48, in middleware
web-1  |     return get_response(request)
web-1  |            ^^^^^^^^^^^^^^^^^^^^^
web-1  |   File "/usr/local/lib/python3.12/site-packages/asgiref/sync.py", line 187, in __call__
web-1  |     raise RuntimeError(
web-1  | RuntimeError: You cannot use AsyncToSync in the same thread as an async event loop - just await the async function directly.

My MIDDLEWARE setting looks like this:

MIDDLEWARE = [
    'orthocal.middleware.request_queueing',
    'corsheaders.middleware.CorsMiddleware',
    'servestatic.middleware.ServeStaticMiddleware',
    'django.middleware.gzip.GZipMiddleware',
    'django.contrib.sessions.middleware.SessionMiddleware',
    'django.middleware.locale.LocaleMiddleware',
    'django.middleware.common.CommonMiddleware',
    'google.cloud.logging_v2.handlers.middleware.RequestMiddleware',
    'orthocal.middleware.cache_control',
]

If I remove the Google cloud logging middleware, the error goes away. Also, if I downgrade to ServeStatic 1.0.0, the error goes away. I don't know which package is the root cause of the error. I hope to dig into this a little more tonight after work.

@Archmonger
Copy link
Owner Author

I will create a test branch to draft potential solutions to this.

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 this pull request may close these issues.

Warning: StreamingHttpResponse must consume synchronous iterators
2 participants