Add check to call render on the response if it has a render method #11762
Conversation
github-actions
bot
added
DEV: backend
Build Artifacts
|
kolibri/core/decorators.py
Outdated
| @@ -339,7 +339,8 @@ def cache_no_user_data(view_func): | |||
| _response = local() | |||
|
|
|||
| def render_and_cache(response, cache_key): | |||
| response.render() | |||
| if hasattr(response, "render") and callable(response.render): | |||
| response.render() | |||
| etag = hashlib.md5( | |||
| kolibri_version.encode("utf-8") + str(response.content).encode("utf-8") | |||
There was a problem hiding this comment.
For response objects without a render function, is content guaranteed to be defined? Or will it just be null?
I wonder if it might be better to not cache an etag in this case.
There was a problem hiding this comment.
For such a case, we can check if content is defined for response object or not, after checking for the render method. This way, we'll only cache the etag if both the attributes are defined for a particular response object.
There was a problem hiding this comment.
We can check if response.content is defined and not null and then cache the etag, else we can return None for render_and_cache function. Should I proceed with these changes?
There was a problem hiding this comment.
Yes, I think that seems right.
There was a problem hiding this comment.
I did a double check, and Django accepts the etag generating function returning None in the case that an etag does not exist for the resource, which seems right: https://docs.djangoproject.com/en/1.11/topics/conditional-view-processing/#the-condition-decorator
There was a problem hiding this comment.
Thanks for the confirmation :)
I'll move forward with the changes in that case. Will make a commit in some time 👍🏻
|
Sure @rtibbles. Thanks for reviewing this PR |
Summary
This PR fixes the error :
'HttpResponseNotAllowed' object has no attribute 'render'when render method is called on response without checking if render attribute exists or is callable.References
Closes #10506
Testing checklist
PR process
Reviewer checklist
yarnandpip)