Repository navigation
fix(logging): preserve Django request context in async code - #18820
AliAbouelazm wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the Django request middleware to use asgiref.local.Local when available, enabling support for both thread-local and async-local contexts, while falling back to threading.local if asgiref is not installed. It also adds comprehensive unit tests for async and thread-local behaviors. Feedback suggests refactoring a thread-local test to group assertions and middleware execution within a single worker function to prevent potential test flakiness from thread reuse assumptions in ThreadPoolExecutor.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
daniel-sanche
left a comment
There was a problem hiding this comment.
Changes LGTM, but it looks like there are some failing tests to resolve
| from asgiref.local import Local | ||
| except ImportError: | ||
| # Django is optional, so do not require asgiref for other integrations. | ||
| from threading import local as Local |
There was a problem hiding this comment.
If it;s possible to use contextvars directly for this instead, that would be ideal. We try to avoid external dependencies where possible
But that said, this file is specifically for django middleware, and IIUC, django requires asgiref. And you handled the fall-back case. So I think this is fine as-is
Fixes #15413.
Use
asgiref.local.Localwhen available so Django request context is isolated between async tasks and follows sync/async adapters. Keep thethreading.localfallback without adding a dependency.Add regressions for concurrent tasks, both adapter directions, ordinary thread isolation, and the optional-import fallback. Isolate existing request-data tests without clearing the storage object's internals.
Validation:
AI-assisted implementation and validation.