Update reference to headers in the response. #127
Conversation
Signed-off-by: Keith Rothman <537074+litghost@users.noreply.github.com>
There was a problem hiding this comment.
Hi, I came across the issue updating to Django 3.2. It is related (see this comment on the issue) to an upgrade to the HttpResponse headers interface, so changing the logging entries might get rid of the surface problem but won't actually update the library to full 3.2 compatibility. I left some notes in the review, and I'm making my own fork to further investigate the issue. I'll keep you updated!
| @@ -124,7 +124,7 @@ def set_response_headers(response, response_headers): | |||
|
|
|||
| response[header.title()] = value | |||
There was a problem hiding this comment.
The response headers interface got changed in Django 3.2, see this comment on the related issue.
Django ticket: #31789
Related Django Commit: bcc2befd
To make it compatible:
response.headers[header.title()] = value
| @@ -45,7 +45,7 @@ def get_django_response(proxy_response, strict_cookies=False): | |||
| logger.info('Normalizing response headers') | |||
| set_response_headers(response, headers) | |||
|
|
|||
| logger.debug('Response headers: %s', getattr(response, '_headers')) | |||
There was a problem hiding this comment.
headers is now a public attribute (see comment above). We can ditch the getattr and just use response.headers
| @@ -124,7 +124,7 @@ def set_response_headers(response, response_headers): | |||
|
|
|||
| response[header.title()] = value | |||
|
|
|||
| logger.debug('Response headers: %s', getattr(response, '_headers')) | |||
There was a problem hiding this comment.
same as above, ditch the getattr
|
Also, these changes would make the library incompatible with previous versions of Django (<3.2), so some checks should be in place for that (that is why tests are failing after all). My proposal is a check for the |
|
Is there any updates on this issue/pr? |
|
This issue is already fixed and merged via #137. Closing this MR. |
Proposed fix for #126