Repair CI and support current Django (5.1-6.1) - #205
Open
PetrDlouhy wants to merge 2 commits into
Open
Conversation
Two time bombs, both visible on any PR today:
* The workflow has run Python 3.12 since the Django 5.0 update, but
[gh-actions] in tox.ini never learned the mapping - so on 3.12
tox-gh-actions falls back to the bare 'py' env, which has no Django
pin. That installed whatever was newest; since Django 6 released,
'makemigrations --check' demands a BigAutoField migration and the job
fails before a single test runs. Mapping 3.12 to the py312 envs runs
the pinned Django 4.2/5.0 matrix instead.
* Python 3.7 is no longer available on the ubuntu-latest runner images
('Version 3.7 with arch x64 not found'), so that job cannot even set
up. Dropped from the workflow; the py37 tox envs remain for anyone
running tox locally on an interpreter that has it.
|
Warning Review limit reachedNext included review available in 19 minutes. View limit detailsLimit details: You’ve used all 2 included reviews currently available. This review ran on the open-source allowance, not this organization's plan, because the pull request author doesn't have an assigned seat. Waiting won't change this — ask an organization admin to assign them a seat, or add seats in Billing if every seat is already assigned, then retry. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
Comment |
Two real incompatibilities surfaced once the resolver was allowed to install modern Django: * Django 6.1 gave SessionBase a __bool__, so an *empty* session is now falsy. Two truthiness checks changed meaning: _get_participant demoted every fresh visitor to a DummyUser (no enrollment, nothing counted), and _session_key returned None for them, keying every such visitor's enrollments and counters to the same identifier - the test suite's MultipleObjectsReturned came from exactly that collision. Both are now identity checks. * Django >= 6 defaults DEFAULT_AUTO_FIELD to BigAutoField, which made makemigrations demand an id migration from every project. The app now pins its historical AutoField in the AppConfig, so existing installations are not asked to alter their tables. The tox envlist grows django5.1/5.2 (py310-313) and django6.0/6.1 (py312-313), the workflow matrix gains Python 3.13, and [gh-actions] learns the 3.13 mapping. Verified locally on Django 5.0.14, 5.1.15, 5.2.17, 6.0.8 and 6.1: makemigrations --check clean and the full suite OK on each.
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
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.
Independent of #204, but found through it. Two layers, one commit each.
Commit 1 — repair what's red on every PR today
tox (3.12)— the workflow has run Python 3.12 since #202, but[gh-actions]intox.iniwas never taught the mapping, so 3.12 falls back to the bare unpinnedpyenv.It installed whatever Django was newest; since Django 6 released,
makemigrations --checkfails before a single test runs.
tox (3.7)— ubuntu-latest runners no longer shipPython 3.7 (
Version 3.7 with arch x64 not found); dropped from the workflow, the localtox envs stay.
Commit 2 — support current Django, extend the matrix
Everything the matrix tested (2.2–5.0) is past end-of-life; the currently supported
Djangos are 5.2 LTS, 6.0 and 6.1. Letting the resolver install them surfaced two real
package bugs, not just config:
SessionBasea__bool__— an empty session is now falsy. Twotruthiness checks changed meaning:
_get_participantdemoted every fresh visitor to aDummyUser(nothing enrolled, nothing counted), and_session_keyreturnedNoneforthem, keying every such visitor's enrollments and counters to the same identifier —
the suite's
MultipleObjectsReturnedwas that collision. Both are identity checks now.Without this, the package silently stops counting new visitors on Django 6.1.
DEFAULT_AUTO_FIELDtoBigAutoField, makingmakemigrationsdemand an
Alter field id on enrollmentmigration from every project. The AppConfig nowpins the historical
AutoField, so existing installations aren't asked to rewrite theirtables. (Whether the project wants to move to
BigAutoFieldeventually is a realquestion — but that's a schema decision for a maintainer, not a side effect of CI.)
Matrix:
django5.1/5.2on py310–313,django6.0/6.1on py312–313, Python 3.13 added tothe workflow and
[gh-actions].Verification
Full suite run locally on Django 5.0.14, 5.1.15, 5.2.17, 6.0.8 and 6.1 —
makemigrations --checkclean and all tests OK on each.