Database migrations

[easybe]

goosebit/.gitignore

Line 168 in 21975c0

/migrations

Shouldn't migrations be committed? Am I missing something here?

Unfortunately, the commit message of 6e7c328 is not very informative 😞

[tsagadar]

To my understanding we removed it until the schema becomes somewhat stable. Then it has to be re-introduced.

[b-rowan]

To my understanding we removed it until the schema becomes somewhat stable. Then it has to be re-introduced.

Pretty much. Migrations are implemented, but it seems like half the time they don't work properly and won't allow the application to start (at least with sqlite). So the idea is to try to stabilize the schema as much as possible so hopefully they actually work when small changes are needed.

[easybe]

We should fix this for the next release. It is a major issue for our setup.

[b-rowan]

We should fix this for the next release. It is a major issue for our setup.

I'm not sure if that's because we don't have migrations in git, or because migrations get generated locally and applied on startup. For me, I end up having to delete them every so often because they stop the app from starting at all.

Seems like mostly an issue with SQLite not having some functionality for migrations, and aerich not being completely fully implemented yet...

[easybe]

With my Docker image, built with migrations/ in .dockerignore (used together with PostgreSQL) I see errors like this:

goosebit-1  | [2024-08-27 14:25:24 +0000] [7] [ERROR] Traceback (most recent call last):
goosebit-1  |   File "/usr/local/lib/python3.12/site-packages/starlette/routing.py", line 732, in lifespan
goosebit-1  |     async with self.lifespan_context(app) as maybe_state:
goosebit-1  |   File "/usr/local/lib/python3.12/contextlib.py", line 210, in __aenter__
goosebit-1  |     return await anext(self.gen)
goosebit-1  |            ^^^^^^^^^^^^^^^^^^^^^
goosebit-1  |   File "/usr/local/lib/python3.12/site-packages/goosebit/__init__.py", line 23, in lifespan
goosebit-1  |     await db.init()
goosebit-1  |   File "/usr/local/lib/python3.12/site-packages/goosebit/db.py", line 23, in init
goosebit-1  |     await command.migrate()
goosebit-1  |   File "/usr/local/lib/python3.12/site-packages/aerich/__init__.py", line 127, in migrate
goosebit-1  |     return await Migrate.migrate(name)
goosebit-1  |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^
goosebit-1  |   File "/usr/local/lib/python3.12/site-packages/aerich/migrate.py", line 140, in migrate
goosebit-1  |     cls.diff_models(cls._last_version_content, new_version_content)
goosebit-1  |   File "/usr/local/lib/python3.12/site-packages/aerich/migrate.py", line 193, in diff_models
goosebit-1  |     old_models.pop(_aerich, None)
goosebit-1  |     ^^^^^^^^^^^^^^
goosebit-1  | AttributeError: 'NoneType' object has no attribute 'pop'

Manually removing the migrations directory seemed to have worked before. Now, I get the following error:

goosebit-1  | [2024-08-27 14:33:48 +0000] [8] [ERROR] Traceback (most recent call last):
goosebit-1  |   File "/usr/local/lib/python3.12/pathlib.py", line 1311, in mkdir
goosebit-1  |     os.mkdir(self, mode)
goosebit-1  | FileNotFoundError: [Errno 2] No such file or directory: '/usr/local/lib/python3.12/site-packages/goosebit/migrations/models'
goosebit-1  | 
goosebit-1  | During handling of the above exception, another exception occurred:
goosebit-1  | 
goosebit-1  | Traceback (most recent call last):
goosebit-1  |   File "/usr/local/lib/python3.12/site-packages/starlette/routing.py", line 732, in lifespan
goosebit-1  |     async with self.lifespan_context(app) as maybe_state:
goosebit-1  |   File "/usr/local/lib/python3.12/contextlib.py", line 210, in __aenter__
goosebit-1  |     return await anext(self.gen)
goosebit-1  |            ^^^^^^^^^^^^^^^^^^^^^
goosebit-1  |   File "/usr/local/lib/python3.12/site-packages/goosebit/__init__.py", line 23, in lifespan
goosebit-1  |     await db.init()
goosebit-1  |   File "/usr/local/lib/python3.12/site-packages/goosebit/db.py", line 21, in init
goosebit-1  |     await command.init_db(safe=True)
goosebit-1  |   File "/usr/local/lib/python3.12/site-packages/aerich/__init__.py", line 133, in init_db
goosebit-1  |     dirname.mkdir(parents=True)
goosebit-1  |   File "/usr/local/lib/python3.12/pathlib.py", line 1316, in mkdir
goosebit-1  |     self.mkdir(mode, parents=False, exist_ok=exist_ok)
goosebit-1  |   File "/usr/local/lib/python3.12/pathlib.py", line 1311, in mkdir
goosebit-1  |     os.mkdir(self, mode)
goosebit-1  | FileExistsError: [Errno 17] File exists: '/usr/local/lib/python3.12/site-packages/goosebit/migrations/models'

Having migration files as part of the package and preventing them from being generated at start-up seems to me like the way to go. Or is that not how aerich is supposed to be used?

[b-rowan]

Having migration files as part of the package and preventing them from being generated at start-up seems to me like the way to go. Or is that not how aerich is supposed to be used?

Not sure. My goal would be that when we make changes to the DB, the user updates goosebit, then runs it, and it generates migrations which then get applied, as each user's migrations in theory could be different.

[easybe]

I haven't had time to look into aerich, but, I know how e.g. Django's migrations work (which I understand is aerich's inspiration).

Besides the initial migrations, they only need to be created when models (actually the DB schema) are modified and they should be written in a DB-agnostic manner. So, it should be possible to ship them with the package and provide a way for smooth DB migrations on updates. Other frameworks work the same, so I would really assume that this is the case here too.

If the files in the migrations directory are only intermediate, temporary files (which I doubt they are), they cannot live in site-packages/goosebit/migrations and the path to the directory would need to be changed to a "temporary" location.

[easybe]

For what it's worth, this is what ChatGPT spits out when asked "Can I get an example of a migration for an edge case that works with PostgreSQL, MySQL and SQLite?":

from tortoise import BaseDBAsyncClient

async def upgrade(db: BaseDBAsyncClient) -> None:
    # Determine the database type
    dialect = db.schema_generator.client.capabilities.dialect

    if dialect == "postgres":
        # PostgreSQL: Use native JSONB type
        await db.execute_script("""
        ALTER TABLE your_table_name
        ADD COLUMN json_data JSONB;
        """)
    elif dialect == "mysql":
        # MySQL: Use native JSON type
        await db.execute_script("""
        ALTER TABLE your_table_name
        ADD COLUMN json_data JSON;
        """)
    else:
        # SQLite: Use TEXT to store JSON data
        await db.execute_script("""
        ALTER TABLE your_table_name
        ADD COLUMN json_data TEXT;
        """)

async def downgrade(db: BaseDBAsyncClient) -> None:
    # Drop the JSON field during downgrade
    await db.execute_script("""
    ALTER TABLE your_table_name
    DROP COLUMN json_data;
    """)

Of course, I have no idea if this is correct.

[tsagadar]

I found an interesting project that can create app templates for fastapi / tortoise orm / aerich (and more) projects: /~https://github.com/s3rius/FastAPI-template

Will rework our migrations based on this popular sample. Also think it can give other insights concerning best practices in this space. So worthwhile to have a look at IMHO.