Opened 3 months ago
Closed 3 months ago
#35807 closed Cleanup/optimization (fixed)
Clarify django.urls.set_urlconf scoping behaviour
Reported by: | Enrico Zini | Owned by: | Carlton Gibson |
---|---|---|---|
Component: | Documentation | Version: | 5.0 |
Severity: | Normal | Keywords: | |
Cc: | Andrew Godwin, Carlton Gibson | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
django.urls.set_urlconf
docstring mentions setting the urlconf for the current thread. However, this is backed by asgiref.local.Local
, which is supposed to provide scoping features related to asyncio tasks as well. This becomes relevant, for example, when doing multi-tenancy with more than one urlconf and trying to call django.urls.reverse
in an ASGI application.
I have been trying to infer what is the expected behaviour in async Django code by following the current implementation, and I found that asgiref.local.Local
behaviour has changed over time (see https://github.com/django/asgiref/issues/473).
I assume that using asgiref.local.Local
instead of threading.local
hints at an intention is to give set_urlconf
/get_urlconf
meaningful semantics for Channels consumers or ASGI applications.
Whether the intention is to isolate set_urlconf
/get_urlconf
across different asyncio tasks or to only support isolation between threads, I suppose it would be useful if their behaviour was documented also for the case of asyncio code, especially given they back django.urls.reverse
.
Change History (7)
comment:1 by , 3 months ago
Cc: | added |
---|---|
Resolution: | → needsinfo |
Status: | new → closed |
comment:2 by , 3 months ago
Resolution: | needsinfo → invalid |
---|
Firstly, just for Django, set_urlconf is not public API so you shouldn't use it. Rather prefer the documented HttpRequest.urlconf
attribute (docs) to set per-tenant URLs in a middleware, if that's your strategy.
Secondly, the linked asgiref issue (#473) is in progress, and should be resolved shortly. (Setting locals has been leaking out of asyncio tasks since the recent v3.8, which is a regression. You can pin to v3.7.2 pending the next release.) It's not a Django issue.
If you need further advice please see TicketClosingReasons/UseSupportChannels.
comment:3 by , 3 months ago
From what I could see, "Set the URLconf for the current thread" in set_urlconf
's docstring looks like a leftover from the pre-async times and could become "Set the URLconf for the current thread or asyncio task".
I understand it's not a public API and indeed I only set request.urlconf
in my code. We spotted the docstring while trying to double check the validity of what we were designing on the async case, and had some extra digging to do to reassure ourselves we were still doing the right thing, hence this issue.
I'm surprised the ticket got closed, but fair enough.
comment:4 by , 3 months ago
Resolution: | invalid |
---|---|
Status: | closed → new |
Triage Stage: | Unreviewed → Accepted |
Type: | Uncategorized → Cleanup/optimization |
Yes, OK, just changing the docstring would be fine.
comment:6 by , 3 months ago
Triage Stage: | Accepted → Ready for checkin |
---|
I'm struggling to follow what this is asking for - can you share an example of the behavior you're seeing?
From what I can see, both async and sync requests handle the urlconf the same - it is the ROOT_URLCONF unless set by a middleware as documented.