Adding linting to migrations

.github/workflows/lint-migrations.yml

@@ -0,0 +1,60 @@
+name: Eugene CI check
+
+on: pull_request
+
+env:
+  EUGENE_VERSION: "0.6.1"
+
+permissions:
+  contents: read
+  pull-requests: write
+
+jobs:
+  lint-migration:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          fetch-depth: 0
+      - name: Set up Python 3.11
+        uses: actions/setup-python@v5
+        with:
+          python-version: 3.11.2
+      - uses: actions/cache@v4
+        with:
+          path: ~/.cache/pip
+          key: ${{ runner.os }}-pip-${{ hashFiles('**/requirements.txt') }}
+          restore-keys: |
+            ${{ runner.os }}-pip-
+      - name: Install python dependencies
+        run: |
+          export DISPATCH_LIGHT_BUILD=1
+          python -m pip install --upgrade pip
+          pip install -e ".[dev]"
+      - name: Download eugene
+        run: |
+          curl -L  https://github.com/kaaveland/eugene/releases/download/$EUGENE_VERSION/eugene-x86_64-unknown-linux-musl -o eugene
+          chmod +x eugene
+      - name: Generate sql files
+        run: |
+          export LOG_LEVEL="ERROR"
+          export STATIC_DIR=""
+          export DATABASE_HOSTNAME="localhost"
+          export DATABASE_CREDENTIALS="dispatch:dispatch"
+          export DISPATCH_ENCRYPTION_KEY="NJHDWDJ3PbHT8h"
+          export DISPATCH_JWT_SECRET="foo"
+
+          changed_files=$(git diff --name-only -- src/dispatch/database/revisions/tenant/versions)
+          if [ -z "$changed_files" ]; then
+            echo "No migration files changed, skipping linting"
+            exit 0
+          fi
+          dispatch database upgrade --revision-type tenant --sql
+      - name: Lint files
+        run: ./eugene lint --ignore E9 alembic_output.sql -f md --accept-failures > lint.md
+      - name: Post Comment
+        env:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+        run: |
+          COMMENT=$(cat lint.md)
+          gh pr comment ${{ github.event.pull_request.number }} --body "$COMMENT"

.github/workflows/playwright.yml

@@ -31,7 +31,7 @@ jobs:
       - name: Check out Git repository
         uses: actions/checkout@v3
       - name: Set up Python 3.11
-        uses: actions/setup-python@v2
+        uses: actions/setup-python@v5
         with:
           python-version: 3.11
       - uses: actions/setup-node@v3

.github/workflows/python.yml

@@ -25,10 +25,10 @@ jobs:
       - name: Check out Git repository
         uses: actions/checkout@v2
       - name: Set up Python 3.11
-        uses: actions/setup-python@v2
+        uses: actions/setup-python@v5
         with:
           python-version: 3.11.2
-      - uses: actions/cache@v2
+      - uses: actions/cache@v4
         with:
           path: ~/.cache/pip
           key: ${{ runner.os }}-pip-${{ hashFiles('**/requirements.txt') }}

src/dispatch/cli.py

@@ -396,6 +396,8 @@ def drop_database(yes):
 def upgrade_database(tag, sql, revision, revision_type):
     """Upgrades database schema to newest version."""
     import sqlalchemy
+    from contextlib import redirect_stdout
+
     from alembic import command as alembic_command
     from alembic.config import Config as AlembicConfig
     from sqlalchemy import inspect
@@ -405,32 +407,41 @@ def upgrade_database(tag, sql, revision, revision_type):
     from .database.manage import init_database
 
     alembic_cfg = AlembicConfig(config.ALEMBIC_INI_PATH)
+    path = None
+    if revision_type:
+        if revision_type == "core":
+            path = config.ALEMBIC_CORE_REVISION_PATH
 
-    if not database_exists(str(config.SQLALCHEMY_DATABASE_URI)):
-        click.secho("Found no database to upgrade, initializing new database...")
-        init_database(engine)
-    else:
-        conn = engine.connect()
-
-        # detect if we need to convert to a multi-tenant schema structure
-        schema_names = inspect(engine).get_schema_names()
-        if "dispatch_core" not in schema_names:
-            click.secho("Detected single tenant database, converting to multi-tenant...")
-            conn.execute(sqlalchemy.text(open(config.ALEMBIC_MULTI_TENANT_MIGRATION_PATH).read()))
+        elif revision_type == "tenant":
+            path = config.ALEMBIC_TENANT_REVISION_PATH
 
-        if revision_type:
-            if revision_type == "core":
-                path = config.ALEMBIC_CORE_REVISION_PATH
+    alembic_cfg.set_main_option("script_location", path)
 
-            elif revision_type == "tenant":
-                path = config.ALEMBIC_TENANT_REVISION_PATH
+    # doesn't support core and tenant at the same time
+    if sql:
+        with open("alembic_output.sql", "w") as sql_file:
+            with redirect_stdout(sql_file):
+                alembic_command.upgrade(alembic_cfg, revision, sql=sql, tag=tag)
 
-            alembic_cfg.set_main_option("script_location", path)
-            alembic_command.upgrade(alembic_cfg, revision, sql=sql, tag=tag)
+    else:
+        if not database_exists(str(config.SQLALCHEMY_DATABASE_URI)):
+            click.secho("Found no database to upgrade, initializing new database...")
+            init_database(engine)
         else:
-            for path in [config.ALEMBIC_CORE_REVISION_PATH, config.ALEMBIC_TENANT_REVISION_PATH]:
-                alembic_cfg.set_main_option("script_location", path)
+            conn = engine.connect()
+
+            # detect if we need to convert to a multi-tenant schema structure
+            schema_names = inspect(engine).get_schema_names()
+            if "dispatch_core" not in schema_names:
+                click.secho("Detected single tenant database, converting to multi-tenant...")
+                conn.execute(sqlalchemy.text(open(config.ALEMBIC_MULTI_TENANT_MIGRATION_PATH).read()))
+
+            if path:
                 alembic_command.upgrade(alembic_cfg, revision, sql=sql, tag=tag)
+            else:
+                for path in [config.ALEMBIC_CORE_REVISION_PATH, config.ALEMBIC_TENANT_REVISION_PATH]:
+                    alembic_cfg.set_main_option("script_location", path)
+                    alembic_command.upgrade(alembic_cfg, revision, sql=sql, tag=tag)
 
     click.secho("Success.", fg="green")
 

src/dispatch/database/revisions/core/env.py

@@ -1,4 +1,4 @@
-from alembic import context
+from alembic import context, script
 from sqlalchemy import engine_from_config, pool
 
 from dispatch.logging import logging
@@ -45,7 +45,10 @@ def process_revision_directives(context, revision, directives):
             log.info("No changes found skipping revision creation.")
 
     connectable = engine_from_config(
-        config.get_section(config.config_ini_section), prefix="sqlalchemy.", poolclass=pool.NullPool
+        config.get_section(config.config_ini_section),
+        prefix="sqlalchemy.",
+        poolclass=pool.NullPool,
+        connect_args={'options': '-c lock_timeout=4000 -c statement_timeout=5000'}
     )
 
     log.info("Migrating dispatch core schema...")
@@ -58,14 +61,51 @@ def process_revision_directives(context, revision, directives):
             target_metadata=target_metadata,
             include_schemas=True,
             include_object=include_object,
+            transaction_per_migration=True,
             process_revision_directives=process_revision_directives,
         )
 
         with context.begin_transaction():
             context.run_migrations()
 
 
+def run_migrations_offline():
+    """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.
+
+    """
+
+    # Get the URL from the config
+    url = config.get_main_option("sqlalchemy.url")
+
+    alembic_script = script.ScriptDirectory.from_config(config)
+
+    _, previous = list(alembic_script.walk_revisions(base='base', head='heads'))[:2]
+
+    # Set the migration options
+    context.configure(
+        url=url,
+        target_metadata=target_metadata,
+        include_schemas=True,
+        include_object=include_object,
+        literal_binds=True,  # Binds parameters with their string values
+        starting_rev=previous.revision,
+    )
+
+    # Start a transaction and run migrations
+    with context.begin_transaction():
+        context.run_migrations()
+
+
+
 if context.is_offline_mode():
-    log.info("Can't run migrations offline")
+    run_migrations_offline()
 else:
     run_migrations_online()

src/dispatch/database/revisions/tenant/env.py

@@ -1,4 +1,5 @@
-from alembic import context
+from alembic import context, script
+
 from sqlalchemy import engine_from_config, pool, inspect
 
 
@@ -51,7 +52,10 @@ def process_revision_directives(context, revision, directives):
             log.info("No changes found skipping revision creation.")
 
     connectable = engine_from_config(
-        config.get_section(config.config_ini_section), prefix="sqlalchemy.", poolclass=pool.NullPool
+        config.get_section(config.config_ini_section),
+        prefix="sqlalchemy.",
+        poolclass=pool.NullPool,
+        connect_args={'options': '-c lock_timeout=4000 -c statement_timeout=5000'}
     )
 
     with connectable.connect() as connection:
@@ -66,6 +70,7 @@ def process_revision_directives(context, revision, directives):
                 target_metadata=target_metadata,
                 include_schemas=True,
                 include_object=include_object,
+                transaction_per_migration=True,
                 process_revision_directives=process_revision_directives,
             )
 
@@ -77,7 +82,42 @@ def process_revision_directives(context, revision, directives):
                     break
 
 
+def run_migrations_offline():
+    """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.
+
+    """
+
+    # Get the URL from the config
+    url = config.get_main_option("sqlalchemy.url")
+
+    alembic_script = script.ScriptDirectory.from_config(config)
+
+    _, previous = list(alembic_script.walk_revisions(base='base', head='heads'))[:2]
+
+    # Set the migration options
+    context.configure(
+        url=url,
+        target_metadata=target_metadata,
+        include_schemas=True,
+        include_object=include_object,
+        literal_binds=True,  # Binds parameters with their string values
+        starting_rev=previous.revision,
+    )
+
+    # Start a transaction and run migrations
+    with context.begin_transaction():
+        context.run_migrations()
+
+
 if context.is_offline_mode():
-    log.info("Can't run migrations offline")
+    run_migrations_offline()
 else:
     run_migrations_online()