Migrate database driver from psycopg2 to psycopg3#4114
Draft
adobloug wants to merge 10 commits into
Draft
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #4114 +/- ##
==========================================
- Coverage 64.99% 64.97% -0.02%
==========================================
Files 627 627
Lines 47096 47101 +5
==========================================
- Hits 30609 30603 -6
- Misses 16487 16498 +11 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
psycopg2 is in maintenance mode; NAV must move to psycopg3 to stay on current Python and Django and to gain access to Django's native connection pooling. psycopg3 does not support PostgreSQL's ±infinity timestamps out of the box, which NAV relies on for unresolved events (end_time stored as 'infinity'). Use the django-psycopg-infinity backend, which registers psycopg3 loaders mapping ±infinity to datetime.max/min while otherwise behaving as Django's standard PostgreSQL backend. psycopg2 is kept as a transitional dependency so the legacy nav.db callers that still import it keep working while they are migrated one by one; it is removed in the final migration commit. hstore is still registered automatically by django.contrib.postgres on the psycopg3 backend, so no manual registration is needed. Refs Uninett#3190. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
EventQ used the legacy nav.db connection cache (getConnection) and psycopg2's set_isolation_level to run event posting in a single transaction. That connection cache is the shared, non-thread-safe path behind the connection-leak races (Uninett#4104), and set_isolation_level does not exist in psycopg3. Use django.db.connection (thread-local, pooled via CONN_MAX_AGE) and wrap the multi-statement post in transaction.atomic() instead of manual isolation-level and commit handling. Cursors are now managed with context managers so they are always closed. While here, load all event variables in consume_events with fetchall(); the previous fetchmany() call loaded only a single variable per event. Refs Uninett#3190. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
psycopg3 drops the psycopg2 async API (conn.poll() and the conn.notifies list) that the eventengine and ipdevpoll notification readers relied on. Under the psycopg3 backend both daemons crashed at runtime with "AttributeError: 'Connection' object has no attribute 'poll'". Add nav.db.get_pg_notifications(), a non-blocking drain built on the libpq primitives consume_input()/notifies(), and route both readers through it. consume_input() raises OperationalError on a dropped connection, preserving the reconnect behaviour poll() used to trigger. Also fix a pre-existing format-string bug in ipdevpoll's empty-connection log message (two %r, one argument). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
navclean and mailin used nav.db.getConnection with manual commit/rollback; move them onto django.db.connection and transaction.atomic(). navclean's dry-run now rolls back via transaction.set_rollback() instead of an explicit rollback, and catches django.db errors since the Django cursor wraps the underlying psycopg exceptions. Reimplement nav.db.escape() on top of psycopg3's sql.Literal, so navsynctypes and navclean can keep embedding literals in generated SQL text after psycopg2 is removed. Refs Uninett#3190 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
snmptrapd, its trap handlers, smsd and logengine used nav.db.getConnection with manual isolation, commit and rollback. Move them onto django.db.connection and transaction.atomic(). Django connections are autocommit by default, so the rollback hacks that kept now() fresh (smsd) and the per-handler rollback in snmptrapd become unnecessary; multi-statement writes are wrapped in transaction.atomic() instead. Legacy psycopg exception catches (nav.db.driver.*) are replaced with their django.db equivalents. smsd's _connect() now resets an unusable Django connection via close_if_unusable_or_obsolete() and retries, preserving the reconnect behaviour it previously got from retry_on_db_loss. After daemonizing, snmptrapd drops the inherited connection so Django opens a fresh one in the forked process. Refs Uninett#3190 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
statemon keeps its own threaded connection (a single connection shared across threads under a lock), which does not fit Django's thread-local connection model, so it stays on a raw psycopg connection rather than moving to django.db. Swap the psycopg2 API for psycopg3: psycopg.connect() (READ COMMITTED is the driver default, so the explicit set_isolation_level call is dropped), connection.closed instead of .status, psycopg.errors.InFailedSqlTransaction instead of decoding InternalError.pgcode, and psycopg.IntegrityError/InterfaceError. psycopg3 cursors expose no .query attribute, so log the statement being run instead. Refs Uninett#3190 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
pgsync (schema bootstrap, runs before the ORM is usable) and radiusparser (runs on the radius server without any NAV libraries) keep their own raw connections. Swap psycopg2 for psycopg3: psycopg.connect(), and the psycopg2 DataError/ProgrammingError/ OperationalError catches for their psycopg3 equivalents. pgsync drops the explicit set_isolation_level(READ COMMITTED) call since that is already psycopg3's default for a non-autocommit connection, which is what its explicit commits rely on. radiusparser's set_isolation_level(0) becomes autocommit = True. The radius web report's server-side named cursor already goes through django.db and needs no change: psycopg3 creates a server cursor from the same connection.connection.cursor(name=...) call. Refs Uninett#3190 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
psycopg3 loads PostgreSQL inet and cidr columns as Python ipaddress objects by default, whereas psycopg2 returned plain strings. statemon feeds the netbox ip column straight into megaping, whose is_valid_ipv6() calls socket.inet_pton() and requires a str, so pinging crashed with "inet_pton() argument 2 must be str, not IPv4Address". Register psycopg3's TextLoader for inet and cidr on statemon's raw connection to restore the string values its callers expect. Django connections are unaffected; the Django backend already maps these types for its IP fields. Refs Uninett#3190 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Three localized cleanups following review of the psycopg2->psycopg3 migration; no behavioral change to the happy path. - statemon: log query parameters alongside the statement on the debug lines, restoring the visibility psycopg2's cursor.query gave (psycopg3's binary cursor drops it). - statemon: document why psycopg2's InternalError pgcode re-raise branch does not port to psycopg3's per-SQLSTATE exception classes. - navclean: move sys.exit out of the transaction.atomic() block so SystemExit no longer unwinds it on an aborted transaction; the error is captured, rolled back, and reported after the block. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
psycopg3 registers loaders per wire format. The existing statemon fix overrode only the text-format inet/cidr loaders, so results returned over the binary protocol still arrived as ipaddress objects. Under load, pping's first query could use the binary protocol and leak an IPv4Address into megaping, which crashed in inet_pton()/str operations that expect a plain string. Register binary-format loaders that stringify inet/cidr as well, restoring the psycopg2-compatible string contract regardless of wire format. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
adobloug
force-pushed
the
feature/3190-psycopg3-migration
branch
from
July 9, 2026 06:45
3d2f8fc to
e5f58ac
Compare
|
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
2 participants
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.
Fixes #3190.
psycopg2 is in maintenance mode; this migrates NAV's database driver to
psycopg3. The
±infinitytimestamp blocker (NAV stores unresolved events asdatetime.max→'infinity') is solved by thedjango-psycopg-infinitybackend.
Strategy (Django-first): rather than port the legacy
nav.dbconnectioncache to the psycopg3 API, migrate its raw callers onto
django.db.connectionand let Django's psycopg3 backend handle isolation, autocommit, encoding, error
wrapping and pooling. Genuinely-standalone tools (run before/without the ORM)
keep a raw connection with minimal psycopg3 fixes.
What this PR does
psycopg>=3.2+django-psycopg-infinity; switch the DBengine.
psycopg2is retained transitionally (still used by the legacycache callers below) and removed in the follow-up.
django.db.connection+transaction.atomic():event.py,logengine.py,bin/mailin.py,bin/navclean.py,snmptrapd(+ handlers),
smsd/navdbqueue.py.ipdevpoll/db.py,eventengine/engine.py): portedto psycopg3's
pgconn.notifies()via a sharednav.db.get_pg_notificationshelper (psycopg3 dropped psycopg2's
poll()+.notifies).escape()reimplemented on psycopg3sql.Literal(...).as_string().pgsync.py,bin/radiusparser.py,statemon/db.py. statemon registers text loaders forinet/cidr— psycopg3 loads those asipaddressobjects by default, whichbroke megaping (it expects the strings psycopg2 returned).
Deferred to follow-up PRs (avoids overlap)
getConnection/ConnectionObject/_connection_cache/LegacyCleanupMiddleware) + dropping transitionalpsycopg2: separate PR, after the callers still using it land.leaves those files untouched (verified zero conflict with both).
Testing
ruff check+ruff formatclean.Opened as draft — incremental; coordinates with #4111 and #4113.