Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Update to Django 5.1 #2264

Draft
wants to merge 17 commits into
base: main
Choose a base branch
from
Draft

WIP: Update to Django 5.1 #2264

wants to merge 17 commits into from

Conversation

StevenMaude
Copy link
Contributor

@StevenMaude StevenMaude commented Dec 18, 2024

Fixes #2115.

This PR is to:

  • preserve work-in-progress commits from Bump django from 4.2.16 to 5.1.4 #2198
  • …which allows us to close off all the Dependabot Python upgrades, now that we're no longer using Dependabot for those upgrades

@StevenMaude
Copy link
Contributor Author

The issue here is with coding systems test failures, due to the way the tests use databases.

@StevenMaude
Copy link
Contributor Author

This also needs local testing as previously to ensure everything is OK with the upgrade.

@StevenMaude StevenMaude added the deck-scrubbing Tech debt or other between-initiative tidy-up work label Dec 18, 2024
This changes `CheckConstraint.check` to `CheckConstraint.condition`.

See also opensafely-core/job-server@190a645
This results in a warning if not enabled, but will be the default
behaviour in Django 6. Therefore, enable the setting and suppress the
deprecation warning resulting from having that setting enabled (which
will be removed in Django 6).

See https://adamj.eu/tech/2023/12/07/django-fix-urlfield-assume-scheme-warnings/
Review this commit.
There's a global here that's messier, and we could tidy.
There's lots of adding fixtures to the tests, that could be tidier.
Check which fixtures are used here.
@StevenMaude
Copy link
Contributor Author

StevenMaude commented Jan 16, 2025

Current status of this:

  • the working tests do pass, but are a bit of a mess still, so this PR does look somewhat horrendous right now, until we can get everything working, and then refine it
  • the use of pytest fixtures, that those fixtures aren't easily available on classes when you're using unittest.TestCase and, for the base coding system tests, the time at which those fixtures are created, all makes this more complicated
  • it's probably possible to move the general modified TestCase class out of each test module into a higher level, in most cases, and remove that duplication
  • the import_data_path on the TestCase isn't always needed
  • there's a bit of reviewing which pytest fixtures are being used (maybe the settings fixture is needed in places)
  • from what I've looked at of the failing base tests, there's some other magic going on with fixtures (bnf_release and maybe others) that still means that the specified db_alias doesn't get picked up by the test, and we get the same DatabaseOperationForbidden error

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deck-scrubbing Tech debt or other between-initiative tidy-up work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade to Django v5.1.x
1 participant