From 434a8fd786beb0746394d81887947e51cd595f51 Mon Sep 17 00:00:00 2001 From: Russ Allbery Date: Mon, 20 May 2024 15:18:17 -0700 Subject: [PATCH 1/4] Allow database passwords to be SecretStrs When initializing a database engine with create_database_engine or create_sync_session, allow the database password to be a SecretStr instead of a str. This simplifies the call for applications that use Pydantic with SecretStr for secrets. --- changelog.d/20240520_163147_rra_DM_44444.md | 3 +++ docs/user-guide/database.rst | 1 + src/safir/database.py | 9 ++++++--- tests/database_test.py | 9 ++++++--- 4 files changed, 16 insertions(+), 6 deletions(-) create mode 100644 changelog.d/20240520_163147_rra_DM_44444.md diff --git a/changelog.d/20240520_163147_rra_DM_44444.md b/changelog.d/20240520_163147_rra_DM_44444.md new file mode 100644 index 00000000..949896e1 --- /dev/null +++ b/changelog.d/20240520_163147_rra_DM_44444.md @@ -0,0 +1,3 @@ +### New features + +- Allow the database password to be passed to `create_database_engine` and `create_sync_session` as a Pydantic `SecretStr`. diff --git a/docs/user-guide/database.rst b/docs/user-guide/database.rst index 6703d8b6..6a837e62 100644 --- a/docs/user-guide/database.rst +++ b/docs/user-guide/database.rst @@ -70,6 +70,7 @@ For applications using `Click`_ (the recommended way to implement a command-line await engine.dispose() This code assumes that ``main`` is the Click entry point and ``.config`` provides a ``config`` object that contains the settings for the application, including the database URL and password as well as the normal Safir configuration settings. +The database password may be a ``pydantic.SecretStr``, a `str`, or `None` if no password is required by the database. If it receives a connection error from the database, Safir will attempt the initialization five times, two seconds apart, to allow time for networking or a database proxy to start. diff --git a/src/safir/database.py b/src/safir/database.py index c8df5528..db1111a6 100644 --- a/src/safir/database.py +++ b/src/safir/database.py @@ -8,6 +8,7 @@ from typing import overload from urllib.parse import quote, urlparse +from pydantic import SecretStr from sqlalchemy import create_engine from sqlalchemy.exc import OperationalError from sqlalchemy.ext.asyncio import ( @@ -38,7 +39,7 @@ class DatabaseInitializationError(Exception): def _build_database_url( - url: str, password: str | None, *, is_async: bool + url: str, password: str | SecretStr | None, *, is_async: bool ) -> str: """Build the authenticated URL for the database. @@ -66,6 +67,8 @@ def _build_database_url( if is_async and parsed_url.scheme == "postgresql": parsed_url = parsed_url._replace(scheme="postgresql+asyncpg") if password: + if isinstance(password, SecretStr): + password = password.get_secret_value() if not parsed_url.username: raise ValueError(f"No username in database URL {url}") password = quote(password, safe="") @@ -143,7 +146,7 @@ def datetime_to_db(time: datetime | None) -> datetime | None: def create_database_engine( url: str, - password: str | None, + password: str | SecretStr | None, *, isolation_level: str | None = None, ) -> AsyncEngine: @@ -242,7 +245,7 @@ async def create_async_session( def create_sync_session( url: str, - password: str | None, + password: str | SecretStr | None, logger: BoundLogger | None = None, *, isolation_level: str | None = None, diff --git a/tests/database_test.py b/tests/database_test.py index be94bca8..9ca04346 100644 --- a/tests/database_test.py +++ b/tests/database_test.py @@ -8,7 +8,7 @@ import pytest import structlog -from pydantic import BaseModel +from pydantic import BaseModel, SecretStr from sqlalchemy import Column, MetaData, String, Table from sqlalchemy.exc import ProgrammingError from sqlalchemy.future import select @@ -25,7 +25,7 @@ ) TEST_DATABASE_URL = os.environ["TEST_DATABASE_URL"] -TEST_DATABASE_PASSWORD = os.getenv("TEST_DATABASE_PASSWORD") +TEST_DATABASE_PASSWORD = os.environ["TEST_DATABASE_PASSWORD"] Base = declarative_base() @@ -56,7 +56,10 @@ async def test_database_init() -> None: assert result.all() == ["someuser"] await session.remove() - # Reinitializing the database with reset should delete the data. + # Reinitializing the database with reset should delete the data. Try + # passing in the password as a SecretStr. + password = SecretStr(TEST_DATABASE_PASSWORD) + engine = create_database_engine(TEST_DATABASE_URL, password) await initialize_database(engine, logger, schema=Base.metadata, reset=True) session = await create_async_session(engine, logger) async with session.begin(): From df50f95d5d9e644a076be5370a89cbbf6a61fcc8 Mon Sep 17 00:00:00 2001 From: Russ Allbery Date: Mon, 20 May 2024 15:46:18 -0700 Subject: [PATCH 2/4] Drop flake8 from dev dependencies We've switched to Ruff for linting. --- pyproject.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index c9c4f1ce..8d87b8d8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -47,7 +47,6 @@ dev = [ "asgi-lifespan", "coverage[toml]", "fastapi>=0.93.0", - "flake8", "mypy", "pre-commit", "psycopg2", From a72903334142b0da6d6f1c46cc768d10fb389ed1 Mon Sep 17 00:00:00 2001 From: Russ Allbery Date: Mon, 20 May 2024 15:46:53 -0700 Subject: [PATCH 3/4] Relax dependency on documenteer Depend on documenteer>=1 instead of an alpha version so that alpha versions aren't picked up by default. --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 8d87b8d8..defcce66 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -58,7 +58,7 @@ dev = [ "sqlalchemy[mypy]", "uvicorn", # documentation - "documenteer[guide]>=1.0.0a7", + "documenteer[guide]>=1", "autodoc_pydantic", ] gcs = [ From 6c76ca384e9c36d37b6028229bd8d0da8eb749a9 Mon Sep 17 00:00:00 2001 From: Russ Allbery Date: Mon, 20 May 2024 15:50:19 -0700 Subject: [PATCH 4/4] Temporarily pin requests The latest version of requests breaks the docker library. --- .github/workflows/ci.yaml | 3 ++- Makefile | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 9e0ba4f0..1704b39f 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -45,12 +45,13 @@ jobs: steps: - uses: actions/checkout@v4 + # Temporarily pin requests because 2.32.0 breaks docker 6.1.3. - name: Run tox uses: lsst-sqre/run-tox@v1 with: python-version: ${{ matrix.python }} tox-envs: "py,typing" - tox-plugins: "tox-docker" + tox-plugins: "tox-docker 'requests<2.32.0'" docs: diff --git a/Makefile b/Makefile index 7be32eee..93d92929 100644 --- a/Makefile +++ b/Makefile @@ -11,9 +11,10 @@ clean: rm -rf docs/_build rm -rf docs/api +# Temporarily pin requests because 2.32.0 breaks docker 6.1.3. .PHONY: init init: - pip install --upgrade pip tox tox-docker pre-commit + pip install --upgrade pip tox tox-docker pre-commit 'requests<2.32.0' pip install --upgrade -e ".[arq,db,dev,gcs,kubernetes]" pre-commit install rm -rf .tox