Replace cc_delim_re usage with split_header_value for Django 6.1+ compatibility#9978
Replace cc_delim_re usage with split_header_value for Django 6.1+ compatibility#9978laymonage wants to merge 3 commits into
Conversation
| if django.VERSION >= (6, 2): | ||
| # `split_header_value` was added in Django 6.2, replacing the `cc_delim_re` |
There was a problem hiding this comment.
We backported this to 6.1 for inclusion in 6.1 beta, FYI
cc_delim_re is removed in Django 6.1b1+
laymonage
force-pushed
the
fix/cc_delim_re
branch
from
09b5d2d to
eac211c
Compare
laymonage
changed the title
There was a problem hiding this comment.
Pull request overview
Updates DRF’s handling of Vary header parsing to avoid relying on Django’s removed cc_delim_re, restoring compatibility with Django 6.1b1+ by switching to split_header_value (via a DRF compatibility shim).
Changes:
- Replaced
cc_delim_re.split(...)usage inAPIView.finalize_response()withsplit_header_value(...). - Added
split_header_valueimport/fallback shim torest_framework.compat.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| rest_framework/views.py | Switches Vary splitting to split_header_value before calling patch_vary_headers. |
| rest_framework/compat.py | Introduces a Django-version-dependent split_header_value shim. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Add new vary headers to the response instead of overwriting. | ||
| vary_headers = self.headers.pop('Vary', None) | ||
| if vary_headers is not None: | ||
| patch_vary_headers(response, cc_delim_re.split(vary_headers)) | ||
| patch_vary_headers(response, split_header_value(vary_headers)) |
There was a problem hiding this comment.
@laymonage would you mind cross checking this suggestion?
There was a problem hiding this comment.
I couldn't find any tests that exercise the existing behaviour for reference. Unfortunately I don't have time to set it up and write it myself. I can ask AI to write it if that's acceptable here…
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This reverts commit 2667400.
What version of Django will you be supporting in the version of Wagtail that adds 6.1 support? We also have this PR which drops some versions: I think that we should drop older versions before adding new ones, but would be nice to avoid being more aggressive that Wagtail |
Description
cc_delim_reis removed in Django6.2+6.1b1+ (ref: django/django#21438)DRF breaks with the above change in Django, as seen in Wagtail's CI: https://github.com/wagtail/wagtail/actions/runs/27202033860/job/80308185523
Not sure if this is the best way to go about it, just thought I'd send a patch if this might help. Cheers!