[Django] #29673: Thread urlconf isn't reset after response complete
Django
---------------------------------------+------------------------
Reporter: tpict | Owner: nobody
Type: Bug | Status: new
Component: Core (URLs) | Version: 2.1
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
---------------------------------------+------------------------
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 no `urlconf` kwarg, expecting to be given the
urlconf specified by `ROOT_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
--
Ticket URL: <https://code.djangoproject.com/ticket/29673>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: tpict | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by tpict):
* version: 2.1 => 2.0
--
Ticket URL: <https://code.djangoproject.com/ticket/29673#comment:1>
Django
Reporter: tpict | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Carlton Gibson):
Any chance you could put this into a test case?
--
Ticket URL: <https://code.djangoproject.com/ticket/29673#comment:2>
Django
Reporter: tpict | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Keryn Knight):
* cc: Keryn Knight (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/29673#comment:3>
Django
Reporter: tpict | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/29673#comment:4>
Django
Reporter: tpict | Owner: nobody
Type: Bug | Status: closed
Component: Core (URLs) | Version: 2.0
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* status: new => closed
* resolution: => needsinfo
--
Ticket URL: <https://code.djangoproject.com/ticket/29673#comment:5>
Django
Reporter: tpict | Owner: nobody
Type: Bug | Status: closed
Component: Core (URLs) | Version: 2.0
Severity: Normal | Resolution: needsinfo
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by tpict):
Replying to [comment:5 Tim Graham]:
Sorry about the delay. I've just written a test case:
https://github.com/tpict/django-29673-test-case
--
Ticket URL: <https://code.djangoproject.com/ticket/29673#comment:6>
Django
Reporter: tpict | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by tpict):
* status: closed => new
* resolution: needsinfo =>
--
Ticket URL: <https://code.djangoproject.com/ticket/29673#comment:7>
Django
Reporter: tpict | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* version: 2.0 => master
* stage: Unreviewed => Accepted
Comment:
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
[https://github.com/tpict/django-29673-test-
case/blob/21dea785091a65f1b8bd34b13b773d385fa10c10/django29673/primary/middleware.py#L27-L33
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/29673#comment:8>
Django
Reporter: tpict | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Herbert Fortes):
* cc: Herbert Fortes (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/29673#comment:9>
Django
Reporter: tpict | Owner: mthpower
Type: Bug | Status: assigned
Component: Core (URLs) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by mthpower):
* status: new => assigned
* owner: nobody => mthpower
--
Ticket URL: <https://code.djangoproject.com/ticket/29673#comment:10>
Django
Reporter: tpict | Owner: mthpower
Type: Bug | Status: assigned
Component: Core (URLs) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Adam (Chainz) Johnson):
* cc: Adam (Chainz) Johnson (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/29673#comment:11>
Django
Reporter: tpict | Owner: mthpower
Type: Bug | Status: assigned
Component: Core (URLs) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Adam (Chainz) Johnson):
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/29673#comment:12>
Django
Reporter: tpict | Owner: Matthew Power
Type: Bug | Status: closed
Component: Core (URLs) | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"18098d261fe98e0a3b7acbb47152031ff905f6a1" 18098d26]:
{{{
#!CommitTicketReference repository=""
revision="18098d261fe98e0a3b7acbb47152031ff905f6a1"
Fixed #29673 -- Reset the URLconf at the end of each request.
Co-authored-by: Ross Thorne <rmwt...@googlemail.com>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29673#comment:13>