Skip to content

Evict stale connection wrappers after django_db_modify_db_settings - #1320

Open
hossam1244 wants to merge 4 commits into
pytest-dev:mainfrom
hossam1244:fix/1317-stale-db-settings-connection-wrappers
Open

hossam1244 wants to merge 4 commits into
pytest-dev:mainfrom
hossam1244:fix/1317-stale-db-settings-connection-wrappers

Conversation

@hossam1244

Copy link
Copy Markdown

Problem

When django_db_modify_db_settings replaces an entry in settings.DATABASES (the typical Testcontainers pattern), the override can silently fail if anything materialized a DatabaseWrapper earlier — import-time ORM access, an app's AppConfig.ready, or Django itself. Django's ConnectionHandler then keeps serving the stale wrapper, and because its settings cached_property references the old nested dict, setup_databases() connects to the pre-override host. connections.close_all() does not evict wrappers, and connections.databases on modern Django is only a compat property.

Fixes #1317 (full root-cause analysis and workaround in the issue — this PR turns that workaround into the fixture's own behavior).

Solution

In django_db_setup, after django_db_modify_db_settings resolves and before setup_databases():

connections.close_all()
for alias in connections:
    with contextlib.suppress(AttributeError):  # alias never materialized in this thread
        del connections[alias]
connections.__dict__.pop("settings", None)  # cached_property; no public eviction API

In-place mutation of settings.DATABASES[alias][...] was never affected (the cached settings dict is the same object); the eviction matters for replacement — which is what the docs example pattern and every Testcontainers setup do.

Testing

  • tests/test_modify_db_settings.py — regression test via django_pytester: the inner module materializes connections["default"] at import time, then the session fixture replaces the alias dict with a different HOST; the test asserts the connection created by django_db_setup uses the modified host. Fails on main, passes with this change (verified red/green locally).
  • Full suite: 234 passed, 6 skipped (sqlite runner, Django 6.1).

Notes for reviewers

  • connections.__dict__.pop("settings", None) touches where a cached_property stores its value; there is no public API for this eviction. Happy to switch to something more official if one exists / if you'd rather fix it in Django.
  • Deliberately scoped to the documented extension point rather than changing fixture ordering or adding new public API.

django_db_modify_db_settings may replace settings.DATABASES (e.g. to
point at a Testcontainer). When anything materialized a DatabaseWrapper
before that (import-time ORM access, AppConfig.ready), the
ConnectionHandler kept serving the stale wrapper and its cached
settings, so setup_databases connected to the pre-override host.

Close connections, drop the materialized wrappers and clear the
handler's cached settings so connections are re-created from the
possibly modified settings.

Fixes pytest-dev#1317

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

django_db_modify_db_settings can still use stale DATABASES connection wrapper when overriding DB settings for Testcontainers

1 participant