Bug: Models created from UUIDBase cause invalid migrations about litestar HOT 12 CLOSED

Hey, @patrickarmengol,

To use the new UUID columns with alembic, you'll need to make sure that your migrations are aware of the new type.

Here is what my script.py.mako template looks like in a project that I'm currently using the UUID models on:

"""${message}

Revision ID: ${up_revision}
Revises: ${down_revision | comma,n}
Create Date: ${create_date}

"""
import sqlalchemy as sa
from alembic import op

from litestar.contrib.sqlalchemy.types import GUID

${imports if imports else ""}

sa.GUID = GUID 

# revision identifiers, used by Alembic.
revision = ${repr(up_revision)}
down_revision = ${repr(down_revision)}
branch_labels = ${repr(branch_labels)}
depends_on = ${repr(depends_on)}


def upgrade():
    ${upgrades if upgrades else "pass"}


def downgrade():
    ${downgrades if downgrades else "pass"}

from litestar.

Could you also mention which database backend and litestar you are using? I'm not able to reproduce this issue on the main branch.

I was able to define a model, migrations, and apply them without issues.

from litestar.

Thanks for the quick reply. I followed your guidance for script.py.mako, but the id column is still being generated as:

sa.Column('id', litestar.contrib.sqlalchemy.types.GUID(length=16), nullable=False),

instead of:

sa.Column('id', sa.GUID(), nullable=False),

like in your example at https://github.com/cofin/litestar-fullstack/blob/81d33439a87d44ab1dcd93b8d824f83ef37f0658/src/app/lib/db/migrations/versions/2023-05-02_user_teams_8e1731f221c4.py#L33

Perhaps I'm missing a preliminary step? or maybe we are on different versions of sqla?

I'm using postgres+asyncpg, with sqlalchemy v2.0.11 and litestar v2.0.0a7.

from litestar.

I'm working out of my personal project atm, so let me make a public MCVE project for testing.

from litestar.

Here is a simple example project: https://github.com/patrickarmengol/guidstuff

Observe https://github.com/patrickarmengol/guidstuff/blob/master/migrations/versions/fe08b6f6163b_create_whatever_table.py

from litestar.

Thanks for the example project. I was able to reproduce it with this repo.

I think this is due to the enhancement that was put in last week to support binary GUIDs and Oracle databases, and it's just not made it into an official alpha release yet.

If you look at the litestar/contrib/sqlalchemy/types.py that is installed into the .venv, you'll notice that it's different than what's in main. I'm not familiar enough with hatch to tell you what to update, but in poetry, if you set your litestar dependency to this, it'll install from main instead of the last packaged alpha.

litestar = {git = "https://github.com/litestar-org/litestar.git", branch = "main", extras = ["jwt", 'cli', 'redis', 'jinja2', 'sqlalchemy', 'structlog']}

from litestar.

@patrickarmengol Here is the PR that actually modified the __init__ method last week. #1676

from litestar.

@patrickarmengol Here is the PR that actually modified the __init__ method last week. #1676

Oh! It's already fixed. I should've looked more thoroughly before opening this ticket. I'll close this issue out then. Thanks for your help and patience.

from litestar.

Ok, so I did some testing with the main branch of litestar. The generated line is identical to what I have been getting. See https://github.com/patrickarmengol/guidstuff/blob/master/migrations/versions/3e4cd18cb2a2_create_whatever_table.py

sa.Column('id', litestar.contrib.sqlalchemy.types.GUID(length=16), nullable=False),

Ignoring the length bit, I think perhaps my actual issue might be more surface level. Like a misconfiguration or missed step. @cofin how are your migration files generated with sa.GUID() instead of the full litestar.contrib.sqlalchemy.types.GUID() in those columns? I've been looking through the fullstack example project, but can't find anything explicitly affecting it.

from litestar.

@patrickarmengol, I had a bit to look into this today. Here's what I've come up with. Can you test this to see if it works for you?

In your env.py for alembic, can you make the do_run_migrations function look like this?:

def do_run_migrations(connection: Connection) -> None:
    context.configure(connection=connection, target_metadata=target_metadata, compare_type=True, user_module_prefix="sa.")

    with context.begin_transaction():
        context.run_migrations()

I made this one change, deleted your existing migration file and regenerated. It had the sa. prefix added in the migrations once I made these changes.

from litestar.contrib.sqlalchemy.types import GUID
...

sa.GUID = GUID
 

def upgrade() -> None:
    # ### commands auto generated by Alembic - please adjust! ###
    op.create_table('whatever',
    sa.Column('name', sa.String(), nullable=False),
    sa.Column('id', sa.GUID(length=16), nullable=False),
    ...
    )

from litestar.

Also, I forgot to mention, the length attribute you see should be fine in this case. I was able to apply these migrations without any problem.

from litestar.

I tested it out and it generated the same script. It ran fine (though pyright yells at me about "unknown member of module" and I'm not entirely sure why it doesn't fail at runtime with the length arg) and the table in the db looks fine.

I'll roll with this for now. Thanks a bunch.

from litestar.