simonw / simonw/sqlite-utils

migrate --stop-before commits earlier sets before rejecting an already-applied target

Open
#870 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
2.2k
Forks
172
Avg merge
9m
Merged PRs (30d)
1

Description

When sqlite-utils migrate loads multiple migration sets, an already-applied --stop-before target in a later set causes an error after earlier sets have committed pending migrations. The CLI validates that targets exist before applying anything, but the already-applied check happens inside each set's Migrations.apply(), so it comes too late for earlier sets.

The documented --stop-before behavior says an already-applied target is an error “and no pending migrations are applied.”

Reproduction (Python 3.12.12, sqlite-utils 4.2.1 at 85b1be10c81d9dd3567e36faf8dd411e4a8789bd): save this as repro.py and run python repro.py in an environment with sqlite-utils installed.

from pathlib import Path
from tempfile import TemporaryDirectory
from click.testing import CliRunner
from sqlite_utils import Database
from sqlite_utils.cli import cli

with TemporaryDirectory() as tmp:
    root = Path(tmp)
    for name in ("a", "b"):
        (root / f"{name}.py").write_text(
            "from sqlite_utils import Migrations\n"
            f'migrations = Migrations("{name}")\n'
            "@migrations()\n"
            "def first(db):\n"
            f'    db.table("{name}").insert({{"value": 1}})\n'
        )
    db_path = str(root / "test.db")
    runner = CliRunner()
    seed = runner.invoke(cli, ["migrate", db_path, str(root / "b.py")])
    if seed.exit_code != 0:
        raise seed.exception
    result = runner.invoke(cli, [
        "migrate", db_path, str(root / "a.py"), str(root / "b.py"),
        "--stop-before", "b:first",
    ])
    print("Exit code:", result.exit_code)
    print(result.output.strip())
    with Database(db_path) as db:
        print("Earlier pending migration was applied:", db.table("a").exists())
        print("Recorded migrations:", [(r["migration_set"], r["name"]) for r in db.table("_sqlite_migrations").rows])

Output:

Exit code: 1
Error: Cannot stop before migration first in set 'b' - already been applied
Earlier pending migration was applied: True
Recorded migrations: [('b', 'first'), ('a', 'first')]

Expected: the same error, but table a should not exist and only b:first should be recorded. The failed command should leave a:first pending.

The validation in cli.py:migrate could check already-applied targets across all loaded sets before entering the application loop. The current check in Migrations.apply() protects only that individual set.

The reproduction gives the same result on two fresh databases. The existing migration tests pass (31 passed across tests/test_migrations.py and tests/test_cli_migrate.py); the applied-target CLI test currently covers one set.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in cli.py:migrate and compare its validation with Migrations.apply() in migrations.py. Reproduce the multiple-set case, then add coverage alongside the applied-target CLI test in tests/test_cli_migrate.py; done means the command errors before applying the earlier pending migration and only the previously recorded migration remains.

Written by the indexing model from the issue text.

Assessment

Tech stack
python, sqlite
Domain
cli, database
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
85/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.