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 Sarah Boyce, 3 months ago

Cc: Andrew Godwin Carlton Gibson added
Resolution: needsinfo
Status: newclosed

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.

comment:2 by Carlton Gibson, 3 months ago

Resolution: needsinfoinvalid

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.

Last edited 3 months ago by Carlton Gibson (previous) (diff)

comment:3 by Enrico Zini, 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 Carlton Gibson, 3 months ago

Resolution: invalid
Status: closednew
Triage Stage: UnreviewedAccepted
Type: UncategorizedCleanup/optimization

Yes, OK, just changing the docstring would be fine.

comment:5 by Carlton Gibson, 3 months ago

Has patch: set
Owner: set to Carlton Gibson
Status: newassigned

comment:6 by Sarah Boyce, 3 months ago

Triage Stage: AcceptedReady for checkin

comment:7 by Sarah Boyce <42296566+sarahboyce@…>, 3 months ago

Resolution: fixed
Status: assignedclosed

In d876be79:

Fixed #35807 -- Mentioned async case for internal get/set urlconf helpers.

Note: See TracTickets for help on using tickets.
Back to Top