Opened 6 years ago
Closed 6 years ago
#29673 closed Bug (fixed)
Thread urlconf isn't reset after response complete
Reported by: | tpict | Owned by: | Matthew Power |
---|---|---|---|
Component: | Core (URLs) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Keryn Knight, Herbert Fortes, Adam Johnson | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
When setting the urlconf on a request (e.g. in middleware for handling multiple domains pointing to the same Django app), it's not reset until the start of the next request. Since urlconf is threadlocal, this causes problems when running a suite of tests, even if the tests pass when ran individually. For example:
- Django test client makes a request that triggers a middleware to change the urlconf
-
reverse
is called with nourlconf
kwarg, expecting to be given the urlconf specified byROOT_URLCONF
- test throws
NoReverseMatch
I took this problem to the IRC and found that another person recently messaged about the same thing: https://botbot.me/freenode/django/2018-08-07/?msg=103000008&page=3
Change History (13)
comment:1 by , 6 years ago
Version: | 2.1 → 2.0 |
---|
comment:2 by , 6 years ago
comment:3 by , 6 years ago
Cc: | added |
---|
comment:4 by , 6 years ago
There are a number of places in Django's test suite that set request.urlconf
so I think a test is indeed needed to demonstrate the issue.
follow-up: 6 comment:5 by , 6 years ago
Resolution: | → needsinfo |
---|---|
Status: | new → closed |
comment:6 by , 6 years ago
Replying to Tim Graham:
Sorry about the delay. I've just written a test case: https://github.com/tpict/django-29673-test-case
comment:7 by , 6 years ago
Resolution: | needsinfo |
---|---|
Status: | closed → new |
comment:8 by , 6 years ago
Triage Stage: | Unreviewed → Accepted |
---|---|
Version: | 2.0 → master |
Hi Tom.
Thanks for the example project. It's a curious one:
(tmp-d8650c87792fd55) ~/Downloads/django-29673-test-case-master $ ./manage.py test ..E. ====================================================================== ERROR: test_primary (django29673.secondary.tests.SecondaryTestCase) Do stuff with the primary site. ---------------------------------------------------------------------- Traceback (most recent call last): File "/Users/carlton/Downloads/django-29673-test-case-master/django29673/secondary/tests.py", line 12, in test_primary response = self.client.get(reverse("primary-site")) File "/Users/carlton/ve/tmp-d8650c87792fd55/lib/python3.6/site-packages/django/urls/base.py", line 90, in reverse return iri_to_uri(resolver._reverse_with_prefix(view, prefix, *args, **kwargs)) File "/Users/carlton/ve/tmp-d8650c87792fd55/lib/python3.6/site-packages/django/urls/resolvers.py", line 622, in _reverse_with_prefix raise NoReverseMatch(msg) django.urls.exceptions.NoReverseMatch: Reverse for 'primary-site' not found. 'primary-site' is not a valid view function or pattern name. ---------------------------------------------------------------------- Ran 4 tests in 0.017s FAILED (errors=1) (tmp-d8650c87792fd55) ~/Downloads/django-29673-test-case-master $ ./manage.py test django29673.secondary.tests.SecondaryTestCase .. ---------------------------------------------------------------------- Ran 2 tests in 0.009s OK
The workaround you demonstrate in the the middleware.py file is interesting:
# To make tests pass, comment the return statement and uncomment this # block: try: return self.get_response(request) finally: if getattr(request, "urlconf", None) is not None: set_urlconf(None)
If you'd like to push this forward, that would be great. Next step I guess I would be to bring the test case into the Django test suite in a PR.
comment:9 by , 6 years ago
Cc: | added |
---|
comment:10 by , 6 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:11 by , 6 years ago
Cc: | added |
---|
Any chance you could put this into a test case?