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

Refactor coding system database setup in AppConfig.ready() #2289

Open
StevenMaude opened this issue Jan 9, 2025 · 1 comment
Open

Refactor coding system database setup in AppConfig.ready() #2289

StevenMaude opened this issue Jan 9, 2025 · 1 comment

Comments

@StevenMaude
Copy link
Contributor

StevenMaude commented Jan 9, 2025

In Django 5 and up, we get the following warning:

…/django/db/backends/utils.py:98: RuntimeWarning: Accessing the database during app initialization is discouraged. To fix this warning, avoid executing queries in AppConfig.ready() or when your app modules are imported.
  warnings.warn(self.APPS_NOT_READY_WARNING_MSG, category=RuntimeWarning)

This is because of a call in AppConfig.ready().

That call in turn results in the following lines running that access the database:

Commenting these out breaks the code, but suppresses the warning:

def update_coding_system_database_connections():
    """Add the database config for each coding system release"""
    # ensure that the database is ready and the CodingSystemRelease table is available
    # (i.e. migrations have been run)
    # Remove database_ready() check from original code as that access the database:
    # if database_ready():  # pragma: no cover
    # The original code iterates through CodingSystemRelease.objects.all()
    for coding_system_release in []:
        if not CODING_SYSTEMS[coding_system_release.coding_system].has_database:
            continue
        db_path = (
            settings.CODING_SYSTEMS_DATABASE_DIR
            / coding_system_release.coding_system
            / f"{coding_system_release.database_alias}.sqlite3"
        )
        database_dict = {
            **connections.databases[DEFAULT_DB_ALIAS],
            **dj_database_url.parse(f"sqlite:///{db_path}"),
        }
        connections.databases[coding_system_release.database_alias] = database_dict

This is not something that currently breaks anything, but could in future if the warning behaviour is forbidden by Django in future. It's also discouraged by the relevant Django documentation, at least for Django 5.1.

@StevenMaude StevenMaude changed the title Remove coding system database setup from in AppConfig.ready() Refactor coding system database setup in AppConfig.ready() Jan 9, 2025
@StevenMaude StevenMaude added the deck-scrubbing Tech debt or other between-initiative tidy-up work label Jan 9, 2025
@StevenMaude
Copy link
Contributor Author

Options to deal with this from the Django 5.1 upgrade discussion:

  • Rearchitect the coding system databases so that this initialisation is not needed.
  • Rework this code to defer updating connections at a later time.
  • Disable the behaviour in testing.

@StevenMaude StevenMaude removed the deck-scrubbing Tech debt or other between-initiative tidy-up work label Jan 15, 2025
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

No branches or pull requests

1 participant