diff --git a/.github/workflows/lint-migrations.yml b/.github/workflows/lint-migrations.yml new file mode 100644 index 000000000000..20d4acf9df54 --- /dev/null +++ b/.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" diff --git a/.github/workflows/playwright.yml b/.github/workflows/playwright.yml index 1a7897144757..bcf23b462174 100644 --- a/.github/workflows/playwright.yml +++ b/.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 diff --git a/.github/workflows/python.yml b/.github/workflows/python.yml index ed9a547ebc82..2f652f531b7b 100644 --- a/.github/workflows/python.yml +++ b/.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') }} diff --git a/src/dispatch/cli.py b/src/dispatch/cli.py index 02551ba34115..75d946534477 100644 --- a/src/dispatch/cli.py +++ b/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") diff --git a/src/dispatch/database/revisions/core/env.py b/src/dispatch/database/revisions/core/env.py index 0e6967292ec3..364a1489ed63 100644 --- a/src/dispatch/database/revisions/core/env.py +++ b/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,6 +61,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, ) @@ -65,7 +69,43 @@ def process_revision_directives(context, revision, directives): 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() diff --git a/src/dispatch/database/revisions/tenant/env.py b/src/dispatch/database/revisions/tenant/env.py index 0aba8c3ee437..d09b83cd5077 100644 --- a/src/dispatch/database/revisions/tenant/env.py +++ b/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()