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

Not setting the full dialect and driver for CONFIG_DATABASE_URI in case of an external DB currently breaks the docker image #8107

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

sdsys-ch
Copy link

@sdsys-ch sdsys-ch commented Nov 7, 2024

When using PostgreSQL for pgAdmin's config database, SQLAlchemy 2.0+ requires the 'psycopg' dialect vs. 'psycopg2' or 'psycopg3':

>>> from sqlalchemy.dialects.postgresql import psycopg; help(psycopg)
Help on module sqlalchemy.dialects.postgresql.psycopg in sqlalchemy.dialects.postgresql:

NAME
    sqlalchemy.dialects.postgresql.psycopg

DESCRIPTION
    .. dialect:: postgresql+psycopg
        :name: psycopg (a.k.a. psycopg 3)
        :dbapi: psycopg
        :connectstring: postgresql+psycopg://user:password@host:port/dbname[?key=value&key=value...]
        :url: https://pypi.org/project/psycopg/

    ``psycopg`` is the package and module name for version 3 of the ``psycopg``
    database driver, formerly known as ``psycopg2``.  This driver is different
    enough from its ``psycopg2`` predecessor that SQLAlchemy supports it
    via a totally separate dialect; support for ``psycopg2`` is expected to remain
    for as long as that package continues to function for modern Python versions,
    and also remains the default dialect for the ``postgresql://`` dialect
    series.

Problem:

When setting PGADMIN_CONFIG_CONFIG_DATABASE_URI in Docker env to either postgresql://... or postgresql+psycopg3://... we either hit sqlalchemy.exc.NoSuchModuleError: Can't load plugin: sqlalchemy.dialects:postgresql.psycopg3 or ModuleNotFoundError: No module named 'psycopg2'.

This PR ensures proper DBAPI postgresql dialect handling for both initial container validation (via entrypoint) and runtime connections for an external postgres config database.

I chose this route because

  1. Runtime database connections work with all URI formats through Flask-SQLAlchemy
  2. No impact on pgadmin's internal postgres connections via wrapper function

Let me know if you prefer a different approach. Might have overlooked something / duplicate code isn't nice. Thanks.

Christophe Neuerburg and others added 3 commits November 7, 2024 16:28
…3` which it will because `PG_DEFAULT_DRIVER` defaults to it. This is unless `CONFIG_DATABASE_URI` is already correclty set.
Copy link
Contributor

@adityatoshniwal adityatoshniwal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise, changes looks good to me.

web/pgadmin/__init__.py Outdated Show resolved Hide resolved
- add normalize_sqlalchemy_uri() utility function
- support both postgresql:// and postgresql+psycopg3:// URIs
- fix sqlalchemy dialect handling in both startup validation and runtime
- use mapping approach for better maintainability of dialect transformations
- fix documentation and config.py comments
@sdsys-ch sdsys-ch force-pushed the fix-external-config-pgdb-sqlalchemy branch from 8bbfaf3 to 3d2665e Compare November 8, 2024 12:45
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