[deps] Add support for djangorestframework~=3.17.0#348
Open
jcardali wants to merge 5 commits intoopenwisp:masterfrom
Open
[deps] Add support for djangorestframework~=3.17.0#348jcardali wants to merge 5 commits intoopenwisp:masterfrom
jcardali wants to merge 5 commits intoopenwisp:masterfrom
Conversation
📝 WalkthroughWalkthroughThis pull request updates dependency constraints and CI/test configurations to add support for Django REST Framework 3.17. setup.py expands the allowed djangorestframework range from <3.17 to <3.18. tox.ini adds a djangorestframework317 factor and corresponding deps entry. .github/workflows/ci.yml expands the tox matrix to include DRF 3.17 combinations across Python 3.11–3.13 and Django 5.0–5.2, and adjusts existing DRF 3.16/3.15 matrix entries. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/ci.yml (1)
64-72:⚠️ Potential issue | 🟡 MinorDRF 3.16 missing Django 5.2 coverage.
DRF 3.15 has Django 5.2 entries (lines 56-63), but DRF 3.16 does not. For consistency, consider adding Django 5.2 coverage for DRF 3.16 as well, since
tox.inidefines these combinations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 64 - 72, Add Django 5.2 job entries for DRF 3.16 in the CI matrix: update the DRF 3.16 block to include TOXENV values matching tox.ini naming (e.g., py311-django52-djangorestframework316 and py312-django52-djangorestframework316) alongside the existing py311/py312 entries; ensure the python versions align (3.11 and 3.12) and follow the same pattern used for DRF 3.15 so the CI covers Django 5.2 for djangorestframework316.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 73-81: The DRF 3.17 CI matrix is incomplete; update the matrix in
.github/workflows/ci.yml to include all tox environments defined in tox.ini for
DRF 3.17 by adding TOXENV entries for py310-django50-djangorestframework317,
py310-django51-djangorestframework317, py310-django52-djangorestframework317,
py313-django50-djangorestframework317, py313-django51-djangorestframework317,
py313-django52-djangorestframework317, and also add
py311-django52-djangorestframework317 and py312-django52-djangorestframework317
so that all py{310,311,312,313}-django52-djangorestframework317 combinations are
covered; mirror the existing pattern used for TOXENV values around the DRF 3.17
block.
In `@tox.ini`:
- Around line 5-7: The tox envlist currently includes DRF 3.17 test combinations
for Python versions py{310,311,312,313} across Django versions django{50,51,52}
(the pattern lines like
"py{310,311,312,313}-django{50}-djangorestframework{315,316,317}{,-pytest}"
etc.), which is broader than the CI matrix; either narrow the tox envlist to
match CI by removing the django52-djangorestframework317 combinations (and any
Python versions not covered in CI) or expand the CI workflow (ci.yml) to include
the missing combos (e.g., add Django 5.2 and Python 3.10/3.13 entries for DRF
3.17) so tox and CI are aligned. Ensure the change targets the envlist patterns
in tox.ini (the py{...}-django{...}-djangorestframework{...}{,-pytest} entries)
or the CI matrix job definitions to keep both consistent.
---
Outside diff comments:
In @.github/workflows/ci.yml:
- Around line 64-72: Add Django 5.2 job entries for DRF 3.16 in the CI matrix:
update the DRF 3.16 block to include TOXENV values matching tox.ini naming
(e.g., py311-django52-djangorestframework316 and
py312-django52-djangorestframework316) alongside the existing py311/py312
entries; ensure the python versions align (3.11 and 3.12) and follow the same
pattern used for DRF 3.15 so the CI covers Django 5.2 for
djangorestframework316.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f92fe6c6-d377-4efd-8230-414253a61d2a
📒 Files selected for processing (3)
.github/workflows/ci.ymlsetup.pytox.ini
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
- GitHub Check: Python==3.10 | py310-django52-djangorestframework315
- GitHub Check: Python==3.12 | py312-django50-djangorestframework317
- GitHub Check: Python==3.13 | py313-django52-djangorestframework315
- GitHub Check: Python==3.10 | py310-django50-djangorestframework315
- GitHub Check: Python==3.11 | py311-django51-djangorestframework317
- GitHub Check: Python==3.12 | py312-django51-djangorestframework317
- GitHub Check: Python==3.11 | py311-django51-djangorestframework316
- GitHub Check: Python==3.11 | py311-django52-djangorestframework315
- GitHub Check: Python==3.12 | py312-django51-djangorestframework316
- GitHub Check: Python==3.12 | py312-django50-djangorestframework316
- GitHub Check: Python==3.13 | py313-django51-djangorestframework315
- GitHub Check: Python==3.12 | py312-django52-djangorestframework315
- GitHub Check: Python==3.12 | py312-django51-djangorestframework315
- GitHub Check: Python==3.10 | py310-django51-djangorestframework315
- GitHub Check: Python==3.13 | py313-django50-djangorestframework315
- GitHub Check: Python==3.11 | py311-django51-djangorestframework315
- GitHub Check: Python==3.10 | py310-django42-djangorestframework314
- GitHub Check: Python==3.11 | py311-django50-djangorestframework315
- GitHub Check: Python==3.9 | py39-django42-djangorestframework314
🔇 Additional comments (2)
tox.ini (1)
28-28: Dependency mapping for DRF 3.17 is correct.The
~=3.17.0version specifier correctly pins to the 3.17.x series.setup.py (1)
30-30: Version constraint update is correct. The upper bound change from<3.17to<3.18properly allows DRF 3.17.x installations while excluding future 3.18+ versions.
Comment on lines
+73
to
+81
| # DRF 3.17 | ||
| - python: "3.11" | ||
| TOXENV: py311-django52-djangorestframework315 | ||
| TOXENV: py311-django50-djangorestframework317 | ||
| - python: "3.12" | ||
| TOXENV: py312-django52-djangorestframework315 | ||
| - python: "3.13" | ||
| TOXENV: py313-django52-djangorestframework315 | ||
| TOXENV: py312-django50-djangorestframework317 | ||
| - python: "3.11" | ||
| TOXENV: py311-django51-djangorestframework317 | ||
| - python: "3.12" | ||
| TOXENV: py312-django51-djangorestframework317 |
There was a problem hiding this comment.
DRF 3.17 CI matrix coverage is incomplete.
The CI matrix for DRF 3.17 only tests Python 3.11 and 3.12 with Django 5.0 and 5.1. However, tox.ini defines combinations for Python 3.10, 3.11, 3.12, 3.13 across Django 5.0, 5.1, and 5.2.
Missing entries:
py310-django{50,51,52}-djangorestframework317py313-django{50,51,52}-djangorestframework317py{310,311,312,313}-django52-djangorestframework317
Additionally, DRF 3.16 is also missing Django 5.2 coverage while DRF 3.15 has it.
Suggested additions to complete DRF 3.17 coverage
# DRF 3.17
+ - python: "3.10"
+ TOXENV: py310-django50-djangorestframework317
- python: "3.11"
TOXENV: py311-django50-djangorestframework317
- python: "3.12"
TOXENV: py312-django50-djangorestframework317
+ - python: "3.13"
+ TOXENV: py313-django50-djangorestframework317
+ - python: "3.10"
+ TOXENV: py310-django51-djangorestframework317
- python: "3.11"
TOXENV: py311-django51-djangorestframework317
- python: "3.12"
TOXENV: py312-django51-djangorestframework317
+ - python: "3.13"
+ TOXENV: py313-django51-djangorestframework317
+ - python: "3.10"
+ TOXENV: py310-django52-djangorestframework317
+ - python: "3.11"
+ TOXENV: py311-django52-djangorestframework317
+ - python: "3.12"
+ TOXENV: py312-django52-djangorestframework317
+ - python: "3.13"
+ TOXENV: py313-django52-djangorestframework317🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/ci.yml around lines 73 - 81, The DRF 3.17 CI matrix is
incomplete; update the matrix in .github/workflows/ci.yml to include all tox
environments defined in tox.ini for DRF 3.17 by adding TOXENV entries for
py310-django50-djangorestframework317, py310-django51-djangorestframework317,
py310-django52-djangorestframework317, py313-django50-djangorestframework317,
py313-django51-djangorestframework317, py313-django52-djangorestframework317,
and also add py311-django52-djangorestframework317 and
py312-django52-djangorestframework317 so that all
py{310,311,312,313}-django52-djangorestframework317 combinations are covered;
mirror the existing pattern used for TOXENV values around the DRF 3.17 block.
Comment on lines
+5
to
+7
| py{310,311,312,313}-django{50}-djangorestframework{315,316,317}{,-pytest} | ||
| py{310,311,312,313}-django{51}-djangorestframework{315,316,317}{,-pytest} | ||
| py{310,311,312,313}-django{52}-djangorestframework{315,316,317}{,-pytest} |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Tox envlist includes broader coverage than CI matrix.
The tox configuration defines DRF 3.17 tests for Python 3.10, 3.11, 3.12, and 3.13 across Django 5.0, 5.1, and 5.2. However, the CI matrix in ci.yml only tests Python 3.11 and 3.12 with DRF 3.17, and only for Django 5.0 and 5.1 (no Django 5.2 coverage for DRF 3.17).
Consider aligning the CI matrix to test more combinations, or reduce the tox envlist to match what's actually tested in CI.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tox.ini` around lines 5 - 7, The tox envlist currently includes DRF 3.17 test
combinations for Python versions py{310,311,312,313} across Django versions
django{50,51,52} (the pattern lines like
"py{310,311,312,313}-django{50}-djangorestframework{315,316,317}{,-pytest}"
etc.), which is broader than the CI matrix; either narrow the tox envlist to
match CI by removing the django52-djangorestframework317 combinations (and any
Python versions not covered in CI) or expand the CI workflow (ci.yml) to include
the missing combos (e.g., add Django 5.2 and Python 3.10/3.13 entries for DRF
3.17) so tox and CI are aligned. Ensure the change targets the envlist patterns
in tox.ini (the py{...}-django{...}-djangorestframework{...}{,-pytest} entries)
or the CI matrix job definitions to keep both consistent.
auvipy
previously approved these changes
Mar 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Checklist
Reference to Existing Issue
Closes #347
Description of Changes
Declare support for
djangorestframework~=3.17.0Modeled after #334
3.17.0 - https://github.com/encode/django-rest-framework/releases/tag/3.17.0:
I'm unsure if these changes should be made as part of this, or separately.
Screenshot
N/A