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

DM-39484: Fix password auth error creating Postgres registry #845

Merged
merged 1 commit into from
Jun 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions doc/changes/DM-39484.api.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
`Database.fromUri` and `Database.makeEngine` methods now accept `sqlalchemy.engine.URL` instances in addition to strings.
2 changes: 2 additions & 0 deletions doc/changes/DM-39484.bugfix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fixed the bug in initializing PostgreSQL registry which resulted in "password authentication failed" error.
The bug appeared during sqlalchemy 2.0 transition which changed default rendering of URL to string.
4 changes: 2 additions & 2 deletions python/lsst/daf/butler/registries/sql.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ def createFromConfig(

DatabaseClass = config.getDatabaseClass()
database = DatabaseClass.fromUri(
str(config.connectionString), origin=config.get("origin", 0), namespace=config.get("namespace")
config.connectionString, origin=config.get("origin", 0), namespace=config.get("namespace")
)
managerTypes = RegistryManagerTypes.fromConfig(config)
managers = managerTypes.makeRepo(database, dimensionConfig)
Expand Down Expand Up @@ -208,7 +208,7 @@ def fromConfig(
config.replaceRoot(butlerRoot)
DatabaseClass = config.getDatabaseClass()
database = DatabaseClass.fromUri(
config.connectionString.render_as_string(hide_password=False),
config.connectionString,
origin=config.get("origin", 0),
namespace=config.get("namespace"),
writeable=writeable,
Expand Down
4 changes: 3 additions & 1 deletion python/lsst/daf/butler/registry/databases/postgresql.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,9 @@ def __init__(
self._shrinker = NameShrinker(self.dialect.max_identifier_length)

@classmethod
def makeEngine(cls, uri: str, *, writeable: bool = True) -> sqlalchemy.engine.Engine:
def makeEngine(
cls, uri: str | sqlalchemy.engine.URL, *, writeable: bool = True
) -> sqlalchemy.engine.Engine:
return sqlalchemy.engine.create_engine(uri, pool_size=1)

@classmethod
Expand Down
11 changes: 9 additions & 2 deletions python/lsst/daf/butler/registry/databases/sqlite.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,14 +110,18 @@ def makeDefaultUri(cls, root: str) -> Optional[str]:

@classmethod
def makeEngine(
cls, uri: Optional[str] = None, *, filename: Optional[str] = None, writeable: bool = True
cls,
uri: str | sqlalchemy.engine.URL | None = None,
*,
filename: Optional[str] = None,
writeable: bool = True,
) -> sqlalchemy.engine.Engine:
"""Create a `sqlalchemy.engine.Engine` from a SQLAlchemy URI or
filename.

Parameters
----------
uri : `str`
uri : `str` or `sqlalchemy.engine.URL`, optional
A SQLAlchemy URI connection string.
filename : `str`
Name of the SQLite database file, or `None` to use an in-memory
Expand Down Expand Up @@ -147,6 +151,9 @@ def makeEngine(
target = f"file:{filename}"
uri = f"sqlite:///{filename}"
else:
if isinstance(uri, sqlalchemy.engine.URL):
# We have to parse strings anyway, so convert it to string.
uri = uri.render_as_string(hide_password=False)
parsed = urllib.parse.urlparse(uri)
queries = parsed.query.split("&")
if "uri=true" in queries:
Expand Down
15 changes: 11 additions & 4 deletions python/lsst/daf/butler/registry/interfaces/_database.py
Original file line number Diff line number Diff line change
Expand Up @@ -252,13 +252,18 @@ def makeDefaultUri(cls, root: str) -> Optional[str]:

@classmethod
def fromUri(
cls, uri: str, *, origin: int, namespace: Optional[str] = None, writeable: bool = True
cls,
uri: str | sqlalchemy.engine.URL,
*,
origin: int,
namespace: str | None = None,
writeable: bool = True,
) -> Database:
"""Construct a database from a SQLAlchemy URI.

Parameters
----------
uri : `str`
uri : `str` or `sqlalchemy.engine.URL`
A SQLAlchemy URI connection string.
origin : `int`
An integer ID that should be used as the default for any datasets,
Expand All @@ -283,12 +288,14 @@ def fromUri(

@classmethod
@abstractmethod
def makeEngine(cls, uri: str, *, writeable: bool = True) -> sqlalchemy.engine.Engine:
def makeEngine(
cls, uri: str | sqlalchemy.engine.URL, *, writeable: bool = True
) -> sqlalchemy.engine.Engine:
"""Create a `sqlalchemy.engine.Engine` from a SQLAlchemy URI.

Parameters
----------
uri : `str`
uri : `str` or `sqlalchemy.engine.URL`
A SQLAlchemy URI connection string.
writeable : `bool`, optional
If `True`, allow write operations on the database, including
Expand Down