Table rename does not get recorded in schema snapshot

[chilico]

I've had to rename a table, which works fine:

from piccolo.apps.migrations.auto.migration_manager import MigrationManager

ID = "2024-10-01T12:00:00:000000"
VERSION = "1.19.0"
DESCRIPTION = "Rename OldTable to NewTable"

async def forwards():
    manager = MigrationManager(
        migration_id=ID, app_name="myapp", description=DESCRIPTION
    )
    
    manager.rename_table(
        old_class_name="OldTable",
        old_tablename="old_table",
        new_class_name="NewTable",
        new_tablename="new_table",
        schema=None,
    )
    
    return manager
    
async def backwards():
    manager = MigrationManager(
        migration_id=ID, app_name="myapp", description=DESCRIPTION
    )
    
    manager.rename_table(
        old_class_name="NewTable",
        old_tablename="new_table",
        new_class_name="OldTable",
        new_tablename="old_table",
        schema=None,
    )
    
    return manager

The issue comes when I try and reverse:

> piccolo migrations b myapp 1 --trace
Reverse 1 migration? [y/N] y
  - 2024-10-01T12:00:00:000000 [backwards]... The command failed.
No match was found for NewTable
Traceback (most recent call last):
  File "/usr/local/lib/python3.12/site-packages/targ/__init__.py", line 449, in run
    command.call_with(arg_class)
  File "/usr/local/lib/python3.12/site-packages/targ/__init__.py", line 230, in call_with
    asyncio.run(self.command(**cleaned_kwargs))
  File "/usr/local/lib/python3.12/asyncio/runners.py", line 194, in run
    return runner.run(main)
           ^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/asyncio/runners.py", line 118, in run
    return self._loop.run_until_complete(task)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "uvloop/loop.pyx", line 1517, in uvloop.loop.Loop.run_until_complete
  File "/usr/local/lib/python3.12/site-packages/piccolo/apps/migrations/commands/backwards.py", line 190, in backwards
    response = await run_backwards(
               ^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/piccolo/apps/migrations/commands/backwards.py", line 161, in run_backwards
    return await manager.run()
           ^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/piccolo/apps/migrations/commands/backwards.py", line 115, in run
    return await self.run_migrations_backwards(app_config=app_config)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/piccolo/apps/migrations/commands/backwards.py", line 95, in run_migrations_backwards
    await response.run(backwards=True)
  File "/usr/local/lib/python3.12/site-packages/piccolo/apps/migrations/auto/migration_manager.py", line 976, in run
    await self._run_drop_columns(backwards=backwards)
  File "/usr/local/lib/python3.12/site-packages/piccolo/apps/migrations/auto/migration_manager.py", line 655, in _run_drop_columns
    _Table = await self.get_table_from_snapshot(
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/piccolo/apps/migrations/auto/migration_manager.py", line 407, in get_table_from_snapshot
    diffable_table = await BaseMigrationManager().get_table_from_snapshot(
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/piccolo/apps/migrations/commands/base.py", line 136, in get_table_from_snapshot
    return schema_snapshot.get_table_from_snapshot(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/piccolo/apps/migrations/auto/schema_snapshot.py", line 26, in get_table_from_snapshot
    raise ValueError(f"No match was found for {table_class_name}")
ValueError: No match was found for NewTable

We're early in development and fortunately can drop and create as a workaround, but I want to know if I'm doing something wrong or whether this is an issue that needs fixing.

[sinisaos]

@chilico I can't reproduce it. In my case everything works. You must pass the migration ID in the piccolo migrations backwards your_app_name your_migration_ID. Here is the documentation
Here is result:

(piccolo_env) rkl@mint21:~/dev/piccolo_env/test$ piccolo migrations forwards home
👍 2 migrations already complete
⏩ 1 migration not yet run
🚀 Running 1 migration:
  - 2024-10-04T15:18:21:985721 [forwards]... ok! ✔️
(piccolo_env) rkl@mint21:~/dev/piccolo_env/test$ piccolo migrations check
Listing migrations ...
APP NAME             | MIGRATION_ID               | DESCRIPTION                              | RAN
home                 | 2024-10-04T15:17:31:858521 |                                          | True
home                 | 2024-10-04T15:18:21:985721 |                                          | True <-- after forwards is True
(piccolo_env) rkl@mint21:~/dev/piccolo_env/test$ piccolo migrations backwards home 2024-10-04T15:18:21:985721
Reverse 1 migration? [y/N] y
  - 2024-10-04T15:18:21:985721 [backwards]... ok! ✔️
(piccolo_env) rkl@mint21:~/dev/piccolo_env/test$ piccolo migrations check
Listing migrations ...
APP NAME             | MIGRATION_ID               | DESCRIPTION                              | RAN
home                 | 2024-10-04T15:17:31:858521 |                                          | True
home                 | 2024-10-04T15:18:21:985721 |                                          | False <-- after reversing is False
(piccolo_env) rkl@mint21:~/dev/piccolo_env/test$ piccolo migrations forwards home
👍 2 migrations already complete
⏩ 1 migration not yet run
🚀 Running 1 migration:
  - 2024-10-04T15:18:21:985721 [forwards]... ok! ✔️
(piccolo_env) rkl@mint21:~/dev/piccolo_env/test$ piccolo migrations check
Listing migrations ...
APP NAME             | MIGRATION_ID               | DESCRIPTION                              | RAN
home                 | 2024-10-04T15:17:31:858521 |                                          | True
home                 | 2024-10-04T15:18:21:985721 |                                          | True <-- after forwards is True

[chilico]

Thanks for your reply @sinisaos. I'm aware you can specify the ID for a specific migration, as well as 1 or all. I have been using all of these successfully to rollback migrations throughout out development until I renamed the table, and now I am encountering the schema_snapshot error above. In order to get around this I have had to create a multi step process of creating the new table, migrating data, and dropping, and in reverse. I can then roll back successfully, but not satisfactorily.

[dantownsend]

I wonder if it fails because it interacts with a previous migration in some weird way.

There's no need to have a backwards function. I know it seems intuitive to do so, because there's a forwards function. But when Piccolo goes backwards it reads the forwards function, and undoes all of it.

[sinisaos]

@chilico I don't know what the problem is in your case. In my case everything works after table name changes.

Logs

(piccolo_env) rkl@mint21:~/dev/piccolo_env/test$ piccolo migrations new home --auto

                              HOME                              
----------------------------------------------------------------
🚀 Creating new migration ...
Did you rename NewTable (tablename: new_table) to OldTable (tablename: old_table)? (y/N)y
Created tables                           0
Dropped tables                           0
Renamed tables                           1
Tables which changed schema              0
Created table columns                    0
Dropped columns                          0
Columns added to existing tables         0
Renamed columns                          0
Altered columns                          0

✅ Finished

(piccolo_env) rkl@mint21:~/dev/piccolo_env/test$ piccolo migrations forwards home
👍 3 migrations already complete
⏩ 1 migration not yet run
🚀 Running 1 migration:
  - 2024-10-05T12:28:31:501300 [forwards]... ok! ✔️
(piccolo_env) rkl@mint21:~/dev/piccolo_env/test$ piccolo migrations check
Listing migrations ...
APP NAME             | MIGRATION_ID               | DESCRIPTION                              | RAN
home                 | 2024-10-04T15:17:31:858521 |                                          | True
home                 | 2024-10-04T15:18:21:985721 |                                          | True
home                 | 2024-10-05T12:28:31:501300 |                                          | True
(piccolo_env) rkl@mint21:~/dev/piccolo_env/test$ piccolo migrations backwards home 2024-10-05T12:28:31:501300
Reverse 1 migration? [y/N] y
  - 2024-10-05T12:28:31:501300 [backwards]... ok! ✔️
(piccolo_env) rkl@mint21:~/dev/piccolo_env/test$ piccolo migrations check
Listing migrations ...
APP NAME             | MIGRATION_ID               | DESCRIPTION                              | RAN
home                 | 2024-10-04T15:17:31:858521 |                                          | True
home                 | 2024-10-04T15:18:21:985721 |                                          | True
home                 | 2024-10-05T12:28:31:501300 |                                          | False
(piccolo_env) rkl@mint21:~/dev/piccolo_env/test$ piccolo migrations forwards home
👍 3 migrations already complete
⏩ 1 migration not yet run
🚀 Running 1 migration:
  - 2024-10-05T12:28:31:501300 [forwards]... ok! ✔️
(piccolo_env) rkl@mint21:~/dev/piccolo_env/test$ piccolo migrations check
Listing migrations ...
APP NAME             | MIGRATION_ID               | DESCRIPTION                              | RAN
home                 | 2024-10-04T15:17:31:858521 |                                          | True
home                 | 2024-10-04T15:18:21:985721 |                                          | True
home                 | 2024-10-05T12:28:31:501300 |                                          | True
(piccolo_env) rkl@mint21:~/dev/piccolo_env/test$ piccolo migrations new home --auto

                              HOME                              
----------------------------------------------------------------
🚀 Creating new migration ...
Did you rename OldTable (tablename: old_table) to NewTable (tablename: new_table)? (y/N)y
Created tables                           0
Dropped tables                           0
Renamed tables                           1
Tables which changed schema              0
Created table columns                    0
Dropped columns                          0
Columns added to existing tables         0
Renamed columns                          0
Altered columns                          0

✅ Finished

(piccolo_env) rkl@mint21:~/dev/piccolo_env/test$ piccolo migrations forwards home
👍 4 migrations already complete
⏩ 1 migration not yet run
🚀 Running 1 migration:
  - 2024-10-05T12:30:36:397611 [forwards]... ok! ✔️
(piccolo_env) rkl@mint21:~/dev/piccolo_env/test$ piccolo migrations backwards home 2024-10-05T12:30:36:397611
Reverse 1 migration? [y/N] y
  - 2024-10-05T12:30:36:397611 [backwards]... ok! ✔️
(piccolo_env) rkl@mint21:~/dev/piccolo_env/test$ piccolo migrations check
Listing migrations ...
APP NAME             | MIGRATION_ID               | DESCRIPTION                              | RAN
home                 | 2024-10-04T15:17:31:858521 |                                          | True
home                 | 2024-10-04T15:18:21:985721 |                                          | True
home                 | 2024-10-05T12:28:31:501300 |                                          | True
home                 | 2024-10-05T12:30:36:397611 |                                          | False
(piccolo_env) rkl@mint21:~/dev/piccolo_env/test$ piccolo migrations forwards home
👍 4 migrations already complete
⏩ 1 migration not yet run
🚀 Running 1 migration:
  - 2024-10-05T12:30:36:397611 [forwards]... ok! ✔️
(piccolo_env) rkl@mint21:~/dev/piccolo_env/test$ piccolo migrations check
Listing migrations ...
APP NAME             | MIGRATION_ID               | DESCRIPTION                              | RAN
home                 | 2024-10-04T15:17:31:858521 |                                          | True
home                 | 2024-10-04T15:18:21:985721 |                                          | True
home                 | 2024-10-05T12:28:31:501300 |                                          | True
home                 | 2024-10-05T12:30:36:397611 |                                          | True
(piccolo_env) rkl@mint21:~/dev/piccolo_env/test$ piccolo migrations forwards home
👍 4 migrations already complete
⏩ 1 migration not yet run
🚀 Running 1 migration:
  - 2024-10-05T12:30:36:397611 [forwards]... ok! ✔️
(piccolo_env) rkl@mint21:~/dev/piccolo_env/test$ piccolo migrations check
Listing migrations ...
APP NAME             | MIGRATION_ID               | DESCRIPTION                              | RAN
home                 | 2024-10-04T15:17:31:858521 |                                          | True
home                 | 2024-10-04T15:18:21:985721 |                                          | True
home                 | 2024-10-05T12:28:31:501300 |                                          | True
home                 | 2024-10-05T12:30:36:397611 |                                          | True
(piccolo_env) rkl@mint21:~/dev/piccolo_env/test$ ^C
(piccolo_env) rkl@mint21:~/dev/piccolo_env/test$ piccolo migrations backwards home 2024-10-05T12:30:36:397611
Reverse 1 migration? [y/N] y
  - 2024-10-05T12:30:36:397611 [backwards]... ok! ✔️
(piccolo_env) rkl@mint21:~/dev/piccolo_env/test$ piccolo migrations check
Listing migrations ...
APP NAME             | MIGRATION_ID               | DESCRIPTION                              | RAN
home                 | 2024-10-04T15:17:31:858521 |                                          | True
home                 | 2024-10-04T15:18:21:985721 |                                          | True
home                 | 2024-10-05T12:28:31:501300 |                                          | True
home                 | 2024-10-05T12:30:36:397611 |                                          | False
(piccolo_env) rkl@mint21:~/dev/piccolo_env/test$ piccolo migrations forwards home
👍 4 migrations already complete
⏩ 1 migration not yet run
🚀 Running 1 migration:
  - 2024-10-05T12:30:36:397611 [forwards]... ok! ✔️
(piccolo_env) rkl@mint21:~/dev/piccolo_env/test$ piccolo migrations check
Listing migrations ...
APP NAME             | MIGRATION_ID               | DESCRIPTION                              | RAN
home                 | 2024-10-04T15:17:31:858521 |                                          | True
home                 | 2024-10-04T15:18:21:985721 |                                          | True
home                 | 2024-10-05T12:28:31:501300 |                                          | True
home                 | 2024-10-05T12:30:36:397611 |