From 0b37bd40baf2a346e4dc4ecdbb62aa5f42e752d3 Mon Sep 17 00:00:00 2001 From: Toby Jennings Date: Wed, 11 Dec 2024 09:23:41 -0600 Subject: [PATCH] feat(db): Iterate on Alembic Support custom schema alembic migrations. Update make targets. Deprecate `init` command from service CLI. --- Makefile | 24 +++++++++++----- alembic/README.md | 41 +++++++++++++++++++++++---- alembic/env.py | 48 +++++++++++++++++++++++++++++--- alembic/script.py.mako | 6 ++-- src/lsst/cmservice/cli/server.py | 18 ++++++------ 5 files changed, 108 insertions(+), 29 deletions(-) diff --git a/Makefile b/Makefile index 7cbaacb6..d30e1245 100644 --- a/Makefile +++ b/Makefile @@ -100,7 +100,7 @@ test: export CM_DATABASE_URL=postgresql://cm-service@localhost:${CM_DATABASE_POR test: export CM_DATABASE_PASSWORD=INSECURE-PASSWORD test: export CM_DATABASE_SCHEMA=cm_service_test test: run-compose - python3 -m lsst.cmservice.cli.server init + alembic upgrade head pytest -vvv --asyncio-mode=auto --cov=lsst.cmservice --cov-branch --cov-report=term --cov-report=html ${PYTEST_ARGS} .PHONY: run @@ -109,7 +109,7 @@ run: export CM_DATABASE_URL=postgresql://cm-service@localhost:${CM_DATABASE_PORT run: export CM_DATABASE_PASSWORD=INSECURE-PASSWORD run: export CM_DATABASE_ECHO=true run: run-compose - python3 -m lsst.cmservice.cli.server init + alembic upgrade head python3 -m lsst.cmservice.cli.server run .PHONY: run-worker @@ -118,9 +118,19 @@ run-worker: export CM_DATABASE_URL=postgresql://cm-service@localhost:${CM_DATABA run-worker: export CM_DATABASE_PASSWORD=INSECURE-PASSWORD run-worker: export CM_DATABASE_ECHO=true run-worker: run-compose - python3 -m lsst.cmservice.cli.server init + alembic upgrade head python3 -m lsst.cmservice.daemon +.PHONY: migrate +migrate: export PGUSER=cm-service +migrate: export PGDATABASE=cm-service +migrate: export PGHOST=localhost +migrate: export CM_DATABASE_PORT=$(shell docker compose port postgresql 5432 | cut -d: -f2) +migrate: export CM_DATABASE_PASSWORD=INSECURE-PASSWORD +migrate: export CM_DATABASE_URL=postgresql://${PGHOST}/${PGDATABASE} +migrate: run-compose + alembic upgrade head + #------------------------------------------------------------------------------ # Targets for developers to debug running against local sqlite. Can be used on @@ -130,21 +140,21 @@ run-worker: run-compose .PHONY: test-sqlite test-sqlite: export CM_DATABASE_URL=sqlite+aiosqlite://///test_cm.db test-sqlite: - python3 -m lsst.cmservice.cli.server init + alembic -x cm_database_url=sqlite:///test_cm.db upgrade head pytest -vvv --asyncio-mode=auto --cov=lsst.cmservice --cov-branch --cov-report=term --cov-report=html ${PYTEST_ARGS} .PHONY: run-sqlite run-sqlite: export CM_DATABASE_URL=sqlite+aiosqlite://///test_cm.db run-sqlite: export CM_DATABASE_ECHO=true run-sqlite: - python3 -m lsst.cmservice.cli.server init + alembic -x cm_database_url=sqlite:///test_cm.db upgrade head python3 -m lsst.cmservice.cli.server run .PHONY: run-worker-sqlite run-worker-sqlite: export CM_DATABASE_URL=sqlite+aiosqlite://///test_cm.db run-worker-sqlite: export CM_DATABASE_ECHO=true run-worker-sqlite: - python3 -m lsst.cmservice.cli.server init + alembic -x cm_database_url=sqlite:///test_cm.db upgrade head python3 -m lsst.cmservice.daemon @@ -180,7 +190,7 @@ run-usdf-dev: export CM_DATABASE_URL=postgresql://cm-service@${CM_DATABASE_HOST} run-usdf-dev: export CM_DATABASE_PASSWORD=$(shell kubectl --cluster=usdf-cm-dev -n cm-service get secret/cm-pg-app -o jsonpath='{.data.password}' | base64 --decode) run-usdf-dev: export CM_DATABASE_ECHO=true run-usdf-dev: - python3 -m lsst.cmservice.cli.server init + alembic upgrade head python3 -m lsst.cmservice.cli.server run .PHONY: run-worker-usdf-dev diff --git a/alembic/README.md b/alembic/README.md index 6c727e14..8e57ac78 100644 --- a/alembic/README.md +++ b/alembic/README.md @@ -16,13 +16,13 @@ The default database connection URL used by Alembic to create a SQLAlchemy engin a `:memory:` instance of SQLite, which is effectively a no-op. In descending order of preference, the database connection URL can be specified by: -- the `CM_DATABASE_URL` environment variable. - any `-x cm_database_url=...` argument passed to `alembic`. +- the `CM_DATABASE_URL` environment variable. - the value of the `sqlalchemy.url` configuration value in `alembic.ini` (which should be a path to a sqlite database file). With the project's virtual environment installed and activated (`source .venv/bin/activate`), -`alembic` is availble to invoke at the command line. +`alembic` is available to invoke at the command line. ## Migrating a Database @@ -31,12 +31,23 @@ most recent migration, and `alembic downgrade base` will do the opposite, effect destroying all Alembic-managed database resources. Alembic can also run in "offline" mode, which instead of using a connection engine to -affect change in a database generates SQL files that can be manually-applied to a database. -In offline mode, the connection URL is used to flavor the dialect of the generated SQL. +affect change in a database generates SQL files that can be manually applied to a database. +In offline mode, the connection URL is used to flavor the dialect of the generated SQL +instead of creating a database connection. To use offline mode, pass `--sql` as an argument to the `alembic` command; optionally pipe the command's output to a file: `alembic upgrade head --sql > migration.sql`. +The database revisions are applied to a database schema identified by one of these +parameters: + +- `-x schema=...` Alembic command parameter +- `CM_DATABASE_SCHEMA` environment variable (via app config object) +- `"public"` + +The Alembic versions table is stored in the same schema as the database revisions +unless the `-x alembic_schema=...` command parameter is used. + ### Incremental Migrations In development or testing, it may be useful to "step" through the migrations one or more at @@ -54,9 +65,29 @@ existing base model with the Alembic `--autogenerate` option against an empty Po database. Before an autogenerated revision is attempted, an empty Alembic revision is created and applied -to the (empty) database. This initial empty base revision is then updated to contain +to the (empty) database. This initial empty base revision is then backfilled to contain the definitions of any Enum column types detected by Alembic, since these cannot be created automatically. Finally, the instances of Enum columns in the auto-generated revision are modified to include a `create_type=False` parameter. + +## Creating Additional Revisions + +A new revision is created when Alembic is invoked with the `revision` command. The +template file `script.py.mako` is used as a blueprint for the new revision's Python +script. + +A new migration can be created one of two ways: + +1. Make code changes to affect the SQLAlchemy `DeclarativeBase` and use `--autogenerate` + with Alembic. +2. Write Alembic revision and then change code to match. + +In the either case, a new Alembic revision will be most often created by calling + +``` +alembic revision -m MESSAGE [--autogenerate] +``` + +Where `MESSAGE` is a commit-style message describing the revision. diff --git a/alembic/env.py b/alembic/env.py index d9272cbb..e415d465 100644 --- a/alembic/env.py +++ b/alembic/env.py @@ -1,7 +1,8 @@ import os +from logging import getLogger from logging.config import fileConfig -from sqlalchemy import create_engine, pool +from sqlalchemy import create_engine, pool, text import lsst.cmservice.db from alembic import context @@ -24,6 +25,8 @@ # my_important_option = config.get_main_option("my_important_option") # ... etc. +logger = getLogger("alembic") + def run_migrations_offline() -> None: """Run migrations in 'offline' mode. @@ -67,8 +70,8 @@ def run_migrations_online() -> None: and associate a connection with the context. The database engine URI is consumed in descending order from: - - A `CM_DATABASE_URL` environment variable - An `-x cm_database_url=...` CLI argument + - A `CM_DATABASE_URL` environment variable - The value of `sqlalchemy.url` specified in the alembic.ini - A sqlite :memory: database """ @@ -76,8 +79,8 @@ def run_migrations_online() -> None: ( u for u in [ - os.getenv("CM_DATABASE_URL"), context.get_x_argument(as_dictionary=True).get("cm_database_url"), + os.getenv("CM_DATABASE_URL"), config.get_main_option("sqlalchemy.url", default=None), "sqlite://", ] @@ -85,10 +88,47 @@ def run_migrations_online() -> None: ), ) + # Set PG* environment variables from CM_* environment variables to capture + # any that are not part of the URL + os.environ["PGPASSWORD"] = os.getenv("CM_DATABASE_PASSWORD", "") + os.environ["PGPORT"] = os.getenv("CM_DATABASE_PORT", "5432") + connectable = create_engine(url, poolclass=pool.NullPool) + # Build the migration in the schema given on the command line or default to + # the schema built into the metadata. This should be the app config value + # derived from env:CM_DATABASE_SCHEMA. + target_schema = ( + context.get_x_argument(as_dictionary=True).get("schema") or target_metadata.schema or "public" + ) + alembic_schema = context.get_x_argument(as_dictionary=True).get("alembic_schema") or target_schema + + logger.info(f"Using schema {alembic_schema} for alembic revision table") + logger.info(f"Using schema {target_schema} for database revisions") + with connectable.connect() as connection: - context.configure(connection=connection, target_metadata=target_metadata) + if connection.dialect.name == "postgresql": + # Explicit pre-migration schema creation, needed for the schema + # in which the alembic version_table is being created, if different + # the target schema. + connection.execute(text(f"CREATE SCHEMA IF NOT EXISTS {alembic_schema}")) + connection.execute(text(f"CREATE SCHEMA IF NOT EXISTS {target_schema}")) + connection.commit() + # Operations are performed in the first schema on the search_path + connection.execute(text(f"set search_path to {target_schema}")) + connection.commit() + + elif connection.dialect.name == "sqlite": + # SQLite does not care about schema + alembic_schema = None + target_schema = None + + context.configure( + connection=connection, + target_metadata=target_metadata, + version_table_schema=alembic_schema, + include_schemas=False, + ) with context.begin_transaction(): context.run_migrations() diff --git a/alembic/script.py.mako b/alembic/script.py.mako index 501bfde6..838603c2 100644 --- a/alembic/script.py.mako +++ b/alembic/script.py.mako @@ -14,9 +14,9 @@ ${imports if imports else ""} # revision identifiers, used by Alembic. revision: str = ${repr(up_revision)} -down_revision: Union[str, None] = ${repr(down_revision)} -branch_labels: Union[str, Sequence[str], None] = ${repr(branch_labels)} -depends_on: Union[str, Sequence[str], None] = ${repr(depends_on)} +down_revision: str | None = ${repr(down_revision)} +branch_labels: str | Sequence[str] | None = ${repr(branch_labels)} +depends_on: str | Sequence[str] | None = ${repr(depends_on)} def upgrade() -> None: diff --git a/src/lsst/cmservice/cli/server.py b/src/lsst/cmservice/cli/server.py index 4dd8e862..2f1ad821 100644 --- a/src/lsst/cmservice/cli/server.py +++ b/src/lsst/cmservice/cli/server.py @@ -1,11 +1,8 @@ import click -import structlog import uvicorn from safir.asyncio import run_with_asyncio -from safir.database import create_database_engine, initialize_database -from .. import __version__, db -from ..config import config +from .. import __version__ # build the server CLI @@ -15,15 +12,16 @@ def server() -> None: """Administrative command-line interface for cm-service.""" -@server.command() +@server.command(deprecated=True) @click.option("--reset", is_flag=True, help="Delete all existing database data.") @run_with_asyncio async def init(*, reset: bool) -> None: # pragma: no cover - """Initialize the service database.""" - logger = structlog.get_logger(config.logger_name) - engine = create_database_engine(config.database_url, config.database_password) - await initialize_database(engine, logger, schema=db.Base.metadata, reset=reset) - await engine.dispose() + """Initialize the service database. + + .. deprecated:: v1.5.0 + The `init` command is deprecated in v0.2.0; it is replaced by alembic. + """ + print("Use `alembic upgrade head` instead.") @server.command()