Switch from `str` to `EmailStr` errors `alembic revision --autogenerate` with exit code 1 and no stack trace

Cielquan commented 2 years ago

First Check

[X] I added a very descriptive title to this issue.
[X] I used the GitHub search to find a similar issue and didn't find it.
[X] I searched the SQLModel documentation, with the integrated search.
[X] I already searched in Google "How to X in SQLModel" and didn't find any information.
[X] I already read and followed all the tutorial in the docs and didn't find an answer.
[X] I already checked if it is not related to SQLModel but to Pydantic.
[X] I already checked if it is not related to SQLModel but to SQLAlchemy.

Commit to Help

[X] I commit to help with one of those options 👆

Example Code

from pydantic import EmailStr
from sqlmodel import AutoString, Column, Field, SQLModel

class User(SQLModel, table=True):
    """User database model."""

    __tablename__ = "users"

    id: int | None = Field(default=None, primary_key=True)
    email: str
    email2: str = Field(sa_column=Column("email2", AutoString, nullable=False))

Description

When I use the str type alembic runs just fine. But when I change the type to pydantic's EmailStr type alembic errors somehow.

As the type annotation, coming from pydantic, in a SQLModel causes an error with alembic I guess that there is somehow a communication / translation error from SQLModel to alembic.

I looked into the code of sqlmodel and here are my findings:

The function get_sqlachemy_type should return the correct type for EmailStr as it subclasses str
When looking at get_column_from_field all the options setting is skipped when I set sa_column. So for email2 I set a column with a type, but the type annotation still somehow causes the error.

Maybe it is somehow related to #212.

Reproduction steps

Setup alembic like described here
Put code snippet in a python file and import it in alembic's migriations/env.py file
Run alembic revision --autogenerate -m "init"
See no error and successfully created migration script in migrations/versions directory
Change any or both email: str to email: EmailStr / email2: str = ... to email2: EmailStr = ...
Run alembic revision --autogenerate -m "init" again
See alembic exit with exit code 1 and no stack trace

Operating System

Windows

Operating System Details

No response

SQLModel Version

0.0.6

Python Version

3.10.1

Additional Context

No response

Cielquan commented 2 years ago

I somehow get the same error result, when I use a base class and overwrite the type for the Read-Model, which does not make any sense to me.

from pydantic import BaseModel, EmailStr
from sqlmodel import Field, SQLModel

class _UserBase(BaseModel):
    """Base model for User database and data models."""

    email: str

class User(_UserBase, SQLModel, table=True):
    """User database model."""

    __tablename__ = "users"

    id: int | None = Field(default=None, primary_key=True)

class UserRead(_UserBase):
    """User data model for reading database entries."""

    id: int
    email: EmailStr

EDIT: remove noqa comment EDIT2: remove unnecessary foreign key

sashkent3 commented 6 months ago

EmailStr is actually NOT a subclass of str thus the check fails. SQLAlchemy type checks for such types as EmailStr should be added to SQLModel similar to IPV4Address. I can submit a PR if that's ok by maintainers.

sashkent3 commented 6 months ago

Nevermind, I found a draft PR #762, which should probably be linked here.

fastapi / sqlmodel