Added Alembic for migrations

Dockerfile

@@ -22,9 +22,11 @@ RUN python -m venv /home/app/.venv
 ENV PATH="/home/app/.venv/bin:${PATH}"
 
 # Copy and install app
+COPY --chown=app:app migrations /home/app/spoolman/migrations
 COPY --chown=app:app spoolman /home/app/spoolman/spoolman
 COPY --chown=app:app pyproject.toml /home/app/spoolman/
 COPY --chown=app:app requirements.txt /home/app/spoolman/
+COPY --chown=app:app alembic.ini /home/app/spoolman/
 COPY --chown=app:app README.md /home/app/spoolman/
 
 WORKDIR /home/app/spoolman

alembic.ini

@@ -0,0 +1,108 @@
+# A generic, single database configuration.
+
+[alembic]
+# path to migration scripts
+script_location = migrations
+
+# template used to generate migration file names; The default value is %%(rev)s_%%(slug)s
+# Uncomment the line below if you want the files to be prepended with date and time
+file_template = %%(year)d_%%(month).2d_%%(day).2d_%%(hour).2d%%(minute).2d-%%(rev)s_%%(slug)s
+
+# sys.path path, will be prepended to sys.path if present.
+# defaults to the current working directory.
+prepend_sys_path = .
+
+# timezone to use when rendering the date within the migration file
+# as well as the filename.
+# If specified, requires the python-dateutil library that can be
+# installed by adding `alembic[tz]` to the pip requirements
+# string value is passed to dateutil.tz.gettz()
+# leave blank for localtime
+# timezone =
+
+# max length of characters to apply to the
+# "slug" field
+# truncate_slug_length = 40
+
+# set to 'true' to run the environment during
+# the 'revision' command, regardless of autogenerate
+# revision_environment = false
+
+# set to 'true' to allow .pyc and .pyo files without
+# a source .py file to be detected as revisions in the
+# versions/ directory
+# sourceless = false
+
+# version location specification; This defaults
+# to migrations/versions.  When using multiple version
+# directories, initial revisions must be specified with --version-path.
+# The path separator used here should be the separator specified by "version_path_separator" below.
+# version_locations = %(here)s/bar:%(here)s/bat:migrations/versions
+
+# version path separator; As mentioned above, this is the character used to split
+# version_locations. The default within new alembic.ini files is "os", which uses os.pathsep.
+# If this key is omitted entirely, it falls back to the legacy behavior of splitting on spaces and/or commas.
+# Valid values for version_path_separator are:
+#
+# version_path_separator = :
+# version_path_separator = ;
+# version_path_separator = space
+version_path_separator = os  # Use os.pathsep. Default configuration used for new projects.
+
+# set to 'true' to search source files recursively
+# in each "version_locations" directory
+# new in Alembic version 1.10
+# recursive_version_locations = false
+
+# the output encoding used when revision files
+# are written from script.py.mako
+# output_encoding = utf-8
+
+# sqlalchemy.url = driver://user:pass@localhost/dbname
+
+
+[post_write_hooks]
+# post_write_hooks defines scripts or Python functions that are run
+# on newly generated revision scripts.  See the documentation for further
+# detail and examples
+
+# format using "black" - use the console_scripts runner, against the "black" entrypoint
+# hooks = black
+# black.type = console_scripts
+# black.entrypoint = black
+# black.options = -l 79 REVISION_SCRIPT_FILENAME
+
+# Logging configuration
+[loggers]
+keys = root,sqlalchemy,alembic
+
+[handlers]
+keys = console
+
+[formatters]
+keys = generic
+
+[logger_root]
+level = WARN
+handlers = console
+qualname =
+
+[logger_sqlalchemy]
+level = WARN
+handlers =
+qualname = sqlalchemy.engine
+
+[logger_alembic]
+level = INFO
+handlers =
+qualname = alembic
+
+[handler_console]
+class = StreamHandler
+args = (sys.stderr,)
+level = NOTSET
+formatter = generic
+
+[formatter_generic]
+format = %(levelname)-5.5s [%(name)s] %(message)s
+datefmt = %H:%M:%S

migrations/README

@@ -0,0 +1 @@
+Generic single-database configuration with an async dbapi.

migrations/__init__.py

@@ -0,0 +1 @@
+"""Database migrations system."""

migrations/env.py

@@ -0,0 +1,68 @@
+"""Alembic environment file."""
+
+import asyncio
+from logging.config import fileConfig
+
+from alembic import context
+from sqlalchemy.engine import Connection
+
+from spoolman.database.database import Database, get_connection_url
+from spoolman.database.models import Base
+
+config = context.config
+
+if config.config_file_name is not None:
+    fileConfig(config.config_file_name)
+
+target_metadata = Base.metadata
+
+
+def run_migrations_offline() -> None:
+    """Run migrations in 'offline' mode.
+
+    This configures the context with just a URL
+    and not an Engine, though an Engine is acceptable
+    here as well.  By skipping the Engine creation
+    we don't even need a DBAPI to be available.
+
+    Calls to context.execute() here emit the given string to the
+    script output.
+
+    """
+    context.configure(
+        url=get_connection_url(),
+        target_metadata=target_metadata,
+        literal_binds=True,
+        dialect_opts={"paramstyle": "named"},
+    )
+
+    with context.begin_transaction():
+        context.run_migrations()
+
+
+def do_run_migrations(connection: Connection) -> None:
+    """Run migrations in 'online' mode."""
+    context.configure(connection=connection, target_metadata=target_metadata)
+
+    with context.begin_transaction():
+        context.run_migrations()
+
+
+async def run_async_migrations() -> None:
+    """In this scenario we need to create an Engine and associate a connection with the context."""
+    db = Database(get_connection_url())
+    db.connect()
+
+    if db.engine is None:
+        raise RuntimeError("Engine not created.")
+
+    async with db.engine.connect() as connection:
+        await connection.run_sync(do_run_migrations)
+
+    await db.engine.dispose()
+
+
+if context.is_offline_mode():
+    run_migrations_offline()
+else:
+    asyncio.run(run_async_migrations())

migrations/script.py.mako

@@ -0,0 +1,25 @@
+"""${message}-
+
+Revision ID: ${up_revision}
+Revises: ${down_revision | comma,n}
+Create Date: ${create_date}
+"""
+import sqlalchemy as sa
+from alembic import op
+${imports if imports else ""}
+
+# revision identifiers, used by Alembic.
+revision = ${repr(up_revision)}
+down_revision = ${repr(down_revision)}
+branch_labels = ${repr(branch_labels)}
+depends_on = ${repr(depends_on)}
+
+
+def upgrade() -> None:
+    """Perform the upgrade."""
+    ${upgrades if upgrades else "pass"}
+
+
+def downgrade() -> None:
+    """Perform the downgrade."""
+    ${downgrades if downgrades else "pass"}

migrations/versions/2023_05_27_2146-684d32cf7e4d_initial.py

@@ -0,0 +1,83 @@
+"""initial.
+
+Revision ID: 684d32cf7e4d
+Create Date: 2023-05-27 21:46:24.361353
+"""
+import sqlalchemy as sa
+from alembic import op
+from sqlalchemy.engine.reflection import Inspector
+
+# revision identifiers, used by Alembic.
+revision = "684d32cf7e4d"
+down_revision = None
+branch_labels = None
+depends_on = None
+
+
+def upgrade() -> None:
+    """Perform the upgrade."""
+    conn = op.get_bind()
+    inspector = Inspector.from_engine(conn)  # type: ignore[arg-type]
+    tables = inspector.get_table_names()
+    if "vendor" in tables:
+        # If the vendor table exists, we assume that the initial migration has already been performed.
+        return
+
+    op.create_table(
+        "vendor",
+        sa.Column("id", sa.Integer(), nullable=False),
+        sa.Column("registered", sa.DateTime(), nullable=False, server_default=sa.func.now()),
+        sa.Column("name", sa.String(length=64), nullable=False),
+        sa.Column("comment", sa.String(length=1024), nullable=True),
+        sa.PrimaryKeyConstraint("id"),
+    )
+    op.create_index(op.f("ix_vendor_id"), "vendor", ["id"], unique=False)
+    op.create_table(
+        "filament",
+        sa.Column("id", sa.Integer(), nullable=False),
+        sa.Column("registered", sa.DateTime(), nullable=False, server_default=sa.func.now()),
+        sa.Column("name", sa.String(length=64), nullable=True),
+        sa.Column("vendor_id", sa.Integer(), nullable=True),
+        sa.Column("material", sa.String(length=64), nullable=True),
+        sa.Column("price", sa.Float(), nullable=True),
+        sa.Column("density", sa.Float(), nullable=False),
+        sa.Column("diameter", sa.Float(), nullable=False),
+        sa.Column("weight", sa.Float(), nullable=True, comment="The filament weight of a full spool (net weight)."),
+        sa.Column("spool_weight", sa.Float(), nullable=True, comment="The weight of an empty spool."),
+        sa.Column("article_number", sa.String(length=64), nullable=True),
+        sa.Column("comment", sa.String(length=1024), nullable=True),
+        sa.ForeignKeyConstraint(
+            ["vendor_id"],
+            ["vendor.id"],
+        ),
+        sa.PrimaryKeyConstraint("id"),
+    )
+    op.create_index(op.f("ix_filament_id"), "filament", ["id"], unique=False)
+    op.create_table(
+        "spool",
+        sa.Column("id", sa.Integer(), nullable=False),
+        sa.Column("registered", sa.DateTime(), nullable=False, server_default=sa.func.now()),
+        sa.Column("first_used", sa.DateTime(), nullable=True),
+        sa.Column("last_used", sa.DateTime(), nullable=True),
+        sa.Column("filament_id", sa.Integer(), nullable=False),
+        sa.Column("used_weight", sa.Float(), nullable=False),
+        sa.Column("location", sa.String(length=64), nullable=True),
+        sa.Column("lot_nr", sa.String(length=64), nullable=True),
+        sa.Column("comment", sa.String(length=1024), nullable=True),
+        sa.ForeignKeyConstraint(
+            ["filament_id"],
+            ["filament.id"],
+        ),
+        sa.PrimaryKeyConstraint("id"),
+    )
+    op.create_index(op.f("ix_spool_id"), "spool", ["id"], unique=False)
+
+
+def downgrade() -> None:
+    """Perform the downgrade."""
+    op.drop_index(op.f("ix_spool_id"), table_name="spool")
+    op.drop_table("spool")
+    op.drop_index(op.f("ix_filament_id"), table_name="filament")
+    op.drop_table("filament")
+    op.drop_index(op.f("ix_vendor_id"), table_name="vendor")
+    op.drop_table("vendor")

migrations/versions/__init__.py

@@ -0,0 +1 @@
+"""Database migration versions."""

pyproject.toml

@@ -22,6 +22,7 @@ target-version = "py39"
 
 [tool.ruff.per-file-ignores]
 "tests*/*" = ["ANN201", "S101", "PLR2004"]
+"migrations/versions/*" = ["N999"]
 
 [tool.black]
 line-length = 120

requirements.txt

@@ -3,6 +3,7 @@ fastapi==0.95.0
 SQLAlchemy[asyncio,aiomysql,postgresql_asyncpg,aiosqlite]==2.0.15
 pydantic==1.10.7
 platformdirs==3.2.0
+alembic==1.11.1
 
 # CockroachDB support
 sqlalchemy-cockroachdb==2.0.1

spoolman/database/database.py

@@ -2,13 +2,48 @@
 
 import logging
 from collections.abc import AsyncGenerator
+from pathlib import Path
 from typing import Optional
 
+from platformdirs import user_data_dir
 from sqlalchemy import URL
 from sqlalchemy.ext.asyncio import AsyncEngine, AsyncSession, async_sessionmaker, create_async_engine
 
-from spoolman.database.models import Base
+from spoolman import env
-from spoolman.env import get_logging_level
+
+logger = logging.getLogger(__name__)
+
+
+def get_connection_url() -> URL:
+    """Construct the connection URL for the database based on environment variables."""
+    db_type = env.get_database_type()
+    host = env.get_host()
+    port = env.get_port()
+    database = env.get_database()
+    query = env.get_query()
+    username = env.get_username()
+    password = env.get_password()
+
+    if db_type is None:
+        db_type = env.DatabaseType.SQLITE
+
+        data_dir = Path(user_data_dir("spoolman"))
+        data_dir.mkdir(parents=True, exist_ok=True)
+
+        database = str(data_dir.joinpath("spoolman.db"))
+        logger.info('No database type specified, using a default SQLite database located at "%s"', database)
+    else:
+        logger.info('Connecting to database of type "%s" at "%s:%s"', db_type, host, port)
+
+    return URL.create(
+        drivername=db_type.to_drivername(),
+        host=host,
+        port=port,
+        database=database,
+        query=query or {},
+        username=username,
+        password=password,
+    )
 
 
 class Database:
@@ -22,7 +57,7 @@ class Database:
 
     def connect(self: "Database") -> None:
         """Connect to the database."""
-        if get_logging_level() == logging.DEBUG:
+        if env.get_logging_level() == logging.DEBUG:
             logging.getLogger("sqlalchemy.engine").setLevel(logging.INFO)
 
         connect_args = {}
@@ -39,18 +74,11 @@ class Database:
         )
         self.session_maker = async_sessionmaker(self.engine, autocommit=False, autoflush=True)
 
-    async def create_tables(self: "Database") -> None:
-        """Create tables for all defined models."""
-        if self.engine is None:
-            raise RuntimeError("DB is not connected.")
-        async with self.engine.begin() as conn:
-            await conn.run_sync(Base.metadata.create_all)
-
 
 __db: Optional[Database] = None
 
 
-async def setup_db(connection_url: URL) -> None:
+def setup_db(connection_url: URL) -> None:
     """Connect to the database.
 
     Args:
@@ -59,7 +87,6 @@ async def setup_db(connection_url: URL) -> None:
     global __db  # noqa: PLW0603
     __db = Database(connection_url)
     __db.connect()
-    await __db.create_tables()
 
 
 async def get_db_session() -> AsyncGenerator[AsyncSession, None]:

spoolman/env.py

@@ -80,7 +80,7 @@ def get_port() -> Optional[int]:
     try:
         return int(port)
     except ValueError as exc:
-        raise ValueError(f"Failed to parse SPOOLMAN_DB_PORT variable: {str(exc)}") from exc
+        raise ValueError(f"Failed to parse SPOOLMAN_DB_PORT variable: {exc!s}") from exc
 
 
 def get_database() -> Optional[str]:
@@ -109,7 +109,7 @@ def get_query() -> Optional[dict[str, str]]:
         parsed_dict = parse.parse_qs(query, strict_parsing=True)
         return {key: value[0] for key, value in parsed_dict.items()}
     except ValueError as exc:
-        raise ValueError(f"Failed to parse SPOOLMAN_DB_QUERY variable: {str(exc)}") from exc
+        raise ValueError(f"Failed to parse SPOOLMAN_DB_QUERY variable: {exc!s}") from exc
 
 
 def get_username() -> Optional[str]:
@@ -149,7 +149,7 @@ def get_password() -> Optional[str]:
         except OSError as exc:
             raise ValueError(
                 "Failed to parse SPOOLMAN_DB_PASSWORD_FILE variable: "
-                f'Failed to read password from file "{file_path}": {str(exc)}.',
+                f'Failed to read password from file "{file_path}": {exc!s}.',
             ) from exc
 
     # Second attempt: grab directly from an environment variable.

spoolman/main.py

@@ -1,6 +1,7 @@
 """Main entrypoint to the server."""
 
 import logging
+import subprocess
 from pathlib import Path
 
 import uvicorn
@@ -8,11 +9,10 @@ from fastapi import FastAPI
 from fastapi.middleware.cors import CORSMiddleware
 from fastapi.staticfiles import StaticFiles
 from platformdirs import user_data_dir
-from sqlalchemy import URL
 
 from spoolman import env
 from spoolman.api.v1.router import app as v1_app
-from spoolman.database.database import setup_db
+from spoolman.database.database import get_connection_url, setup_db
 
 # Get the data directory
 data_dir = Path(user_data_dir("spoolman"))
@@ -59,38 +59,20 @@ if env.is_debug_mode():
     )
 
 
-def get_connection_url() -> URL:
-    """Construct the connection URL for the database based on environment variables."""
-    db_type = env.get_database_type()
-    host = env.get_host()
-    port = env.get_port()
-    database = env.get_database()
-    query = env.get_query()
-    username = env.get_username()
-    password = env.get_password()
-
-    if db_type is None:
-        db_type = env.DatabaseType.SQLITE
-        database = str(data_dir.joinpath("spoolman.db"))
-        logger.info('No database type specified, using a default SQLite database located at "%s"', database)
-    else:
-        logger.info('Connecting to database of type "%s" at "%s:%s"', db_type, host, port)
-
-    return URL.create(
-        drivername=db_type.to_drivername(),
-        host=host,
-        port=port,
-        database=database,
-        query=query or {},
-        username=username,
-        password=password,
-    )
-
-
 @app.on_event("startup")
 async def startup() -> None:
     """Run the service's startup sequence."""
-    await setup_db(get_connection_url())
+    logger.info("Setting up database...")
+    setup_db(get_connection_url())
+
+    logger.info("Performing migrations...")
+    # Run alembic in a subprocess.
+    # There is some issue with the uvicorn worker that causes the process to hang when running alembic directly.
+    # See: https://github.com/sqlalchemy/alembic/discussions/1155
+    project_root = Path(__file__).parent.parent
+    subprocess.run(["alembic", "upgrade", "head"], check=True, cwd=project_root)  # noqa: S603, S607
+
+    logger.info("Startup complete.")
 
 
 if __name__ == "__main__":

tests_integration/docker-compose-sqlite.yml

@@ -3,7 +3,6 @@ services:
   spoolman:
     image: donkie/spoolman:test
     environment:
-      - SPOOLMAN_DB_TYPE=sqlite
       - SPOOLMAN_LOGGING_LEVEL=DEBUG
   tester:
     image: donkie/spoolman-tester:latest