diff --git a/docs/changelog.rst b/docs/changelog.rst index 0c409a1..1e268ea 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -17,6 +17,7 @@ Unreleased - ``table.upsert()`` and ``table.upsert_all()`` now raise ``PrimaryKeyRequired`` if a record is missing a value for any primary key column, or has a value of ``None`` for one. Previously such records - which can never match an existing row - were quietly inserted as brand new rows, or triggered a confusing ``KeyError`` after the insert had already taken place. - ``db.table(table_name).insert({})`` can now be used to insert a row consisting entirely of default values into an existing table, using ``INSERT INTO ... DEFAULT VALUES``. (:issue:`759`) - ``db.enable_wal()`` and ``db.disable_wal()`` now raise a ``RuntimeError`` if called while a transaction is open. Previously they would silently commit the open transaction as a side effect of changing the journal mode, breaking the rollback guarantee of ``db.atomic()`` and of user-managed transactions. +- Improvements to the ``sqlite-utils migrate`` command: ``--stop-before`` values that do not match any known migration are now an error instead of being silently ignored, ``--stop-before`` now works correctly with migration files that still use the older ``sqlite_migrate.Migrations`` class, and ``--list`` is now a read-only operation that no longer creates the database file or the migrations tracking table. ``migrations.applied()`` now returns migrations in the order they were applied. .. _v4_0rc1: diff --git a/docs/migrations.rst b/docs/migrations.rst index eddac5f..61709c4 100644 --- a/docs/migrations.rst +++ b/docs/migrations.rst @@ -117,7 +117,7 @@ Running the command repeatedly is safe. Migrations that already have a matching Listing migrations ================== -Use ``--list`` to show applied and pending migrations without running them: +Use ``--list`` to show applied and pending migrations without running them. This is a read-only operation - it will not create the database file or the ``_sqlite_migrations`` table: .. code-block:: bash diff --git a/sqlite_utils/cli.py b/sqlite_utils/cli.py index 23f04a7..1c94806 100644 --- a/sqlite_utils/cli.py +++ b/sqlite_utils/cli.py @@ -3387,13 +3387,19 @@ def migrate(db_path, migrations, stop_before, list_, verbose): if not migration_sets: raise click.ClickException("No migrations.py files found") - db = sqlite_utils.Database(db_path) - _register_db_for_cleanup(db) - if list_: + if pathlib.Path(db_path).exists(): + db = sqlite_utils.Database(db_path) + else: + # Listing is read-only - don't create the database file + db = sqlite_utils.Database(memory=True) + _register_db_for_cleanup(db) _display_migration_list(db, migration_sets) return + db = sqlite_utils.Database(db_path) + _register_db_for_cleanup(db) + prev_schema = db.schema if verbose: click.echo("Migrating {}".format(db_path)) diff --git a/sqlite_utils/migrations.py b/sqlite_utils/migrations.py index 20572af..7ae72e6 100644 --- a/sqlite_utils/migrations.py +++ b/sqlite_utils/migrations.py @@ -54,14 +54,10 @@ class Migrations: def pending(self, db: "Database") -> list["Migrations._Migration"]: """ Return a list of pending migrations. + + This is a read-only operation - it does not write to the database. """ - self.ensure_migrations_table(db) - already_applied = { - r["name"] - for r in db[self.migrations_table].rows_where( - "migration_set = ?", [self.name] - ) - } + already_applied = {migration.name for migration in self.applied(db)} return [ migration for migration in self._migrations @@ -70,13 +66,17 @@ class Migrations: def applied(self, db: "Database") -> list["Migrations._AppliedMigration"]: """ - Return a list of applied migrations. + Return a list of applied migrations, in the order they were applied. + + This is a read-only operation - it does not write to the database. """ - self.ensure_migrations_table(db) + table = _table(db, self.migrations_table) + if not table.exists(): + return [] return [ self._AppliedMigration(name=row["name"], applied_at=row["applied_at"]) - for row in db[self.migrations_table].rows_where( - "migration_set = ?", [self.name] + for row in table.rows_where( + "migration_set = ?", [self.name], order_by="rowid" ) ] diff --git a/tests/test_cli_migrate.py b/tests/test_cli_migrate.py index 4a2ca9c..1cdf8e7 100644 --- a/tests/test_cli_migrate.py +++ b/tests/test_cli_migrate.py @@ -426,3 +426,40 @@ def test_stop_before_multiple_values_for_legacy_set_errors(tmpdir): ) assert result.exit_code == 1 assert "single --stop-before" in result.output + + +def test_list_does_not_create_database_file(two_migrations): + path, _ = two_migrations + db_path = path / "test.db" + runner = CliRunner() + result = runner.invoke( + sqlite_utils.cli.cli, ["migrate", str(db_path), str(path), "--list"] + ) + assert result.exit_code == 0, result.output + assert "Pending:\n foo\n bar" in result.output + # Listing migrations must not create the database file + assert not db_path.exists() + + +def test_list_does_not_upgrade_legacy_migrations_table(two_migrations): + path, _ = two_migrations + db_path = str(path / "test.db") + db = sqlite_utils.Database(db_path) + db["_sqlite_migrations"].create( + {"migration_set": str, "name": str, "applied_at": str}, + pk=("migration_set", "name"), + ) + db["_sqlite_migrations"].insert( + {"migration_set": "hello", "name": "foo", "applied_at": "x"} + ) + db.close() + runner = CliRunner() + result = runner.invoke( + sqlite_utils.cli.cli, ["migrate", db_path, str(path), "--list"] + ) + assert result.exit_code == 0, result.output + assert "foo - x" in result.output + # --list must not perform the one-way legacy schema upgrade + db2 = sqlite_utils.Database(db_path) + assert db2["_sqlite_migrations"].pks == ["migration_set", "name"] + db2.close() diff --git a/tests/test_migrations.py b/tests/test_migrations.py index c190fd5..1733aa4 100644 --- a/tests/test_migrations.py +++ b/tests/test_migrations.py @@ -190,3 +190,11 @@ def test_upgrades_sqlite_migrations(migrations, create_table, pk): assert db["_sqlite_migrations"].pks == ([pk] if isinstance(pk, str) else list(pk)) migrations.apply(db) assert db["_sqlite_migrations"].pks == ["id"] + + +def test_pending_and_applied_are_read_only(migrations): + db = sqlite_utils.Database(memory=True) + assert [m.name for m in migrations.pending(db)] == ["m001", "m002"] + assert migrations.applied(db) == [] + # Neither call should have created the tracking table + assert db.table_names() == []