Opened 5 years ago

Closed 5 years ago

#16003 closed Bug (fixed)

Admin incompatible with USE_ETAGS = True

Reported by: Chris Lamb Owned by: nobody
Component: contrib.admin Version: master
Severity: Release blocker Keywords: regression
Cc: pterk@…, real.human@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Karen Tracey)

In SVN, the admin does not work with USE_ETAGS = True:

Environment:

Traceback:
File "/srv/www.example.com/example/packages/django/core/handlers/base.py" in get_response
  111.                         response = callback(request, *callback_args, **callback_kwargs)
File "/srv/www.example.com/example/packages/django/contrib/admin/sites.py" in wrapper
  213.                 return self.admin_view(view, cacheable)(*args, **kwargs)
File "/srv/www.example.com/example/packages/django/utils/decorators.py" in _wrapped_view
  91.                     response = view_func(request, *args, **kwargs)
File "/srv/www.example.com/example/packages/django/views/decorators/cache.py" in _wrapped_view_func
  77.         response = view_func(request, *args, **kwargs)
File "/srv/www.example.com/example/packages/django/contrib/admin/sites.py" in inner
  196.             return view(request, *args, **kwargs)
File "/srv/www.example.com/example/packages/django/views/decorators/cache.py" in _wrapped_view_func
  78.         add_never_cache_headers(response)
File "/srv/www.example.com/example/packages/django/utils/cache.py" in add_never_cache_headers
  116.     patch_response_headers(response, cache_timeout=-1)
File "/srv/www.example.com/example/packages/django/utils/cache.py" in patch_response_headers
  105.         response['ETag'] = '"%s"' % hashlib.md5(response.content).hexdigest()
File "/srv/www.example.com/example/packages/django/template/response.py" in _get_content
  110.             raise ContentNotRenderedError('The response content must be rendered before it can be accessed.')

Exception Type: ContentNotRenderedError at /a/admin/
Exception Value: The response content must be rendered before it can be accessed.

Attachments (2)

16003.patch (873 bytes) - added by Peter van Kampen 5 years ago.
A preliminary patch that fixes the issue in the admin. Needs more work for an alround solution. Also regression tests
16003.2.patch (7.6 KB) - added by Peter van Kampen 5 years ago.
An updated patch /w tests

Download all attachments as: .zip

Change History (11)

comment:1 Changed 5 years ago by Karen Tracey

Description: modified (diff)
Keywords: regression added
Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

Fixed formatting, please use preview. I suspect this is due to r16087. It does not happen with 1.3 release. There's also a likely-related thread on django-developers: http://groups.google.com/group/django-developers/browse_thread/thread/f96e982254fbe5c3

comment:2 Changed 5 years ago by Jannis Leidel

Component: Uncategorizedcontrib.admin

Yeah, this seems like a rather major issue with the TemplateResponse feature, which I think can only be fixed by actually rendering the content if needed instead of raising a ContentNotRenderedError exception.

With proper documentation of which middlewares/decorator modify the content we should be able to keep the usefulness of TemplateResponse up.

comment:3 Changed 5 years ago by Peter van Kampen

Needs tests: set
Patch needs improvement: set
UI/UX: unset

I've got a preliminary fix for this. I'll be at the djangocon sprint in A'dam tomorrow from ~10.00 on to continue work on this.

Changed 5 years ago by Peter van Kampen

Attachment: 16003.patch added

A preliminary patch that fixes the issue in the admin. Needs more work for an alround solution. Also regression tests

comment:4 Changed 5 years ago by Peter van Kampen

Cc: Peter van Kampen added

Changed 5 years ago by Peter van Kampen

Attachment: 16003.2.patch added

An updated patch /w tests

comment:5 Changed 5 years ago by Peter van Kampen

Cc: Peter van Kampen removed
Has patch: set
Needs tests: unset
Patch needs improvement: unset

I think I fixed this for all places patch_response_header is called. I've added a test that fails when USE_ETAGS is set to True and an admin site is called. I've added a few more tests to check various headers with TemplateResponse but I think only the etag is relevant because it requires the content is complete when the etag value is calculated.

The solution, IMO, is using the add_post_render_callback if/when the response is a TemplateResponse (using ducktype checking now rather than using isinstance).

comment:6 Changed 5 years ago by Peter van Kampen

Cc: pterk@… added

comment:7 Changed 5 years ago by Tai Lee

Nevermind. I didn't fully understand the issue.

Last edited 5 years ago by Tai Lee (previous) (diff)

comment:8 Changed 5 years ago by Tai Lee

Cc: real.human@… added

comment:9 Changed 5 years ago by Jannis Leidel

Resolution: fixed
Status: newclosed

In [16729]:

Fixed #16003 -- Restored compatibility of the admin when using USE_ETAGS. Thanks for the initial patch, pterk.

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