Skip to content

Move report queries onto Django's DB connection#4131

Open
adobloug wants to merge 1 commit into
Uninett:5.19.xfrom
adobloug:bugfix/report-django-connection
Open

Move report queries onto Django's DB connection#4131
adobloug wants to merge 1 commit into
Uninett:5.19.xfrom
adobloug:bugfix/report-django-connection

Conversation

@adobloug

@adobloug adobloug commented Jul 9, 2026

Copy link
Copy Markdown

Scope and purpose

Fixes #4130.

The report subsystem read from the database through the legacy
nav.db.getConnection() cache (nav.ObjectCache), which is not thread-safe and
misbehaves under mod_wsgi's multithreaded workers — the same root cause as #4104.
This moves DatabaseResult, IPtree.get_subnets and MetaIP._createMetaMap
onto Django's django.db.connection, whose connections are thread-local, taking
reports off the fragile legacy cache.

metaIP dropped its legacy 'manage' database alias, which already resolved to
the same nav database (no db_manage in db.conf), so that is behaviourally a
no-op.

DB-API errors now surface as django.db.utils.* exceptions rather than
psycopg2.*. dbresult.py is updated to catch ProgrammingError / DataError
from django.db.utils; without this a malformed report SQL definition would
raise uncaught and produce an HTTP 500 — the regression flagged in the review of
#4111.

Split out from #4111 (issue #4104) as a separable change, per review feedback.

How to observe

The new integration tests exercise the real django.db.connection path,
including the error handling:

pytest tests/integration/report/dbresult_test.py \
       tests/integration/report/legacy_query_test.py

test_when_report_sql_is_malformed_then_error_is_set_without_raising and
test_when_report_input_type_is_invalid_then_data_error_is_set fail against the
old psycopg2 handlers (uncaught django.db.utils.* → HTTP 500) and pass with
the Django-wrapped handlers.

Contributor Checklist

  • Added a changelog fragment for towncrier
  • Added/amended tests for new/changed code
  • Added/changed documentation — not applicable, internal change
  • Linted/formatted the code with ruff
  • Wrote the commit message so that the first line continues the sentence "If applied, this commit will ..."
  • Based this pull request on the correct upstream branch (5.19.x, a bugfix affecting the latest stable version)
  • If applicable: Created new issues if this PR does not fix the issue completely — not applicable, fully fixed
  • If it's not obvious from a linked issue, described how to interact with NAV to observe the effects (see above)
  • If this results in changes in the UI: Added screenshots — not applicable, no UI change
  • If this adds a new Python source code file: Added the boilerplate header (both new test files have it)

The report subsystem read from the database through the legacy
nav.db.getConnection() cache (nav.ObjectCache), which is not
thread-safe and misbehaves under mod_wsgi's multithreaded workers
(same root cause as Uninett#4104): cached connections can leak or cross
threads, causing intermittent errors and HTTP 500s.

Migrate DatabaseResult, IPtree.get_subnets and MetaIP._createMetaMap
to django.db.connection, whose connections are thread-local. metaIP
dropped its legacy 'manage' database alias, which already resolved to
the same 'nav' database (no db_manage in db.conf), so this is
behaviourally a no-op.

DB-API errors now surface as django.db.utils.* exceptions rather than
psycopg2.*, so dbresult.py catches ProgrammingError and DataError from
django.db.utils; otherwise a malformed report SQL definition would
raise uncaught and produce an HTTP 500.

Add integration tests that exercise the real connection path, including
the malformed-SQL and invalid-type error handling.

Fixes Uninett#4130.
@sonarqubecloud

sonarqubecloud Bot commented Jul 9, 2026

Copy link
Copy Markdown

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.

2 participants