Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

migration to sqlalchemy 2.0 #563

Closed
wants to merge 23 commits into from
Closed

migration to sqlalchemy 2.0 #563

wants to merge 23 commits into from

Conversation

farahats9
Copy link
Contributor

made the required changes to pass the tests for sqlalchemy 2.0
python version had to be pumped to 3.7 instead of 3.6
removed sqlalchemy2-stubs since it is not compatible or required.

More testing in a real project scenario is welcome.

@github-actions
Copy link

github-actions bot commented Mar 7, 2023

📝 Docs preview for commit 60318b4 at: https://6406825e11187c06ceaecaf0--sqlmodel.netlify.app

@mortendaehli
Copy link

@farahats9, great! Do you think you would be able to sort out the failing tests? :)

@farahats9
Copy link
Contributor Author

I fixed some of the mypy linting errors but I am not good at these things so maybe someone can fix the rest.

@github-actions
Copy link

📝 Docs preview for commit b48423f at: https://641d9f2736a7fa1c9568bdb7--sqlmodel.netlify.app

sqlmodel/sql/expression.py Outdated Show resolved Hide resolved
@honglei
Copy link

honglei commented Mar 31, 2023

I used this version in my project with env: Python3.11.2/sqlalchemy2.0.7/Win10/scheme="postgresql+asyncpg", it works all fine.

@github-actions
Copy link

📝 Docs preview for commit 9c219d9 at: https://6426bd112dc67b078b6e5298--sqlmodel.netlify.app

@github-actions
Copy link

📝 Docs preview for commit 0eee8e8 at: https://6426be4468302c086218a5bb--sqlmodel.netlify.app

docs/contributing.md Outdated Show resolved Hide resolved
docs/tutorial/index.md Show resolved Hide resolved
.github/workflows/test.yml Outdated Show resolved Hide resolved
@github-actions
Copy link

📝 Docs preview for commit 050cf02 at: https://6426c1ce9df2a80bfd37ae93--sqlmodel.netlify.app

@gabrielmbmb
Copy link

@farahats9 someone created a PR on your fork for fixing the mypy issues

@heyaco
Copy link

heyaco commented Apr 20, 2023

This seems good to go just needs merging

@8thgencore
Copy link

We are very much waiting for this change

@antont
Copy link

antont commented Apr 21, 2023

We are very much waiting for this change

Why do you wait, instead of taking it to use? I guess feedback from users would be helpful info also for the maintainer to know when to merge.

@linpan
Copy link

linpan commented Apr 24, 2023

🚀 👀👀

@synodriver
Copy link

Great! I would love to try this branch in my production environment.

@k-s-t-i
Copy link

k-s-t-i commented Apr 24, 2023

Would like to see this merged for asyncpg support. Asyncpg seems to greatly increase performance based on most simple benchmarks.

@farahats9
Copy link
Contributor Author

farahats9 commented Apr 25, 2023

Would like to see this merged for asyncpg support. Asyncpg seems to greatly increase performance based on most simple benchmarks.

@k-s-t-i You can currently use Asyncpg with sqlalchemy v1.4 and sqlmodel v0.0.8 with no issue in most cases. you will need to watch out for some problems like lazy loading and cascading deletes on the orm side.

@synodriver
Copy link

Great! I would love to try this branch in my production environment.

Update: This patch can work with asyncmy in a real project. Will try asyncpg later.

pyproject.toml Outdated Show resolved Hide resolved
@synodriver
Copy link

Great! I would love to try this branch in my production environment.

Update: This patch can work with asyncmy in a real project. Will try asyncpg later.

Update: asyncpg works too. Tested with a real project on both windows and linux. Notice: to make relationship work, you may use Relationship(back_populates="xxx", sa_relationship_kwargs={"lazy": 'selectin'}) where sa_relationship_kwargs is important.

Copy link

@UnoYakshi UnoYakshi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really would like to use it in my new project! Thank you for the PR!

pyproject.toml Outdated Show resolved Hide resolved
sqlmodel/engine/result.py Show resolved Hide resolved
sqlmodel/main.py Show resolved Hide resolved
sqlmodel/sql/expression.py Outdated Show resolved Hide resolved
sqlmodel/sql/expression.py Show resolved Hide resolved
sqlmodel/sql/expression.py Show resolved Hide resolved
sqlmodel/__init__.py Show resolved Hide resolved
@honglei
Copy link

honglei commented Jun 27, 2023

Five months have been passed since the release of SQLalchemy2.0, https://www.sqlalchemy.org/blog/2023/01/26/sqlalchemy-2.0.0-released/

@antont
Copy link

antont commented Jun 27, 2023

Five months have been passed since the release of SQLalchemy2.0, https://www.sqlalchemy.org/blog/2023/01/26/sqlalchemy-2.0.0-released/

You can help by using this PR in your project(s) if you want, and reporting how well it works.

@50Bytes-dev
Copy link

Been working with this PR for almost a month now, works great. Thank you!

@FilipeMarch
Copy link

FilipeMarch commented Jul 10, 2023

Hello @FilipeMarch! I've just change order of classes definition and solve your error:

NameError: name 'Article' is not defined

Full example code (database.py):

from typing import List, Optional
from sqlmodel import SQLModel, Field, Relationship, create_engine

class Article(SQLModel, table=True):
    id: Optional[int] = Field(default=None, primary_key=True)
    reviewer_links: List["ArticleReviewerLink"] = Relationship(
        back_populates="article",
    )

class ArticleReviewerLink(SQLModel, table=True):
    id: Optional[int] = Field(default=None, primary_key=True)
    article_id: Optional[int] = Field(default=None, foreign_key="article.id")
    article: Article = Relationship(back_populates="reviewer_links")

sqlite_file_name = "database.db"
sqlite_url = f"sqlite:///{sqlite_file_name}"

engine = create_engine(sqlite_url, echo=True)

def create_db_and_tables():
    SQLModel.metadata.create_all(engine)

if __name__ == "__main__":
    create_db_and_tables()

Sorry but changing the order of classes definition does not solve the error for me. At this point, I don't know what to do, but this branch is useless for me. I already have done what you mentioned, but the error persists. It always says if raiseerr and "Mapped[" in raw_annotation: # type: ignore TypeError: argument of type 'ForwardRef' is not iterable.

My project has about 30 tables at this point, and it has been running for quite 1 year. My project does not run as is with this branch, and I already tried dozens of things (like switching the order of tables, removing Relationships, removing a bunch of tables) and I can't make it to work in any possible way. Errors keep coming non stop. I can deactivate the Relationships, move tables up or down, it does not matter, new errors come every time. I just feel wasting my time because obviously there is something wrong, but I can't stop the errors until I remove all tables from the code?

The error messages are not helpful at all. And they only happen in this specific PR.

I understand it may be working for a very simple case like you presented, but for me it is always showing these errors, no matter how much code I change:

py", line 2479, in get_property
    return self._props[key]
KeyError: 'user'

or

    annotation = eval(expression, base_globals, locals_)
  File "<string>", line 1, in <module>
NameError: name 'PostUserLike' is not defined

or

.py", line 223, in eval_expression
    raise NameError(
NameError: Could not de-stringify annotation 'PostUserLike'

or

", line 2342, in _extract_mapped_subtype
    if raiseerr and "Mapped[" in raw_annotation:  # type: ignore
TypeError: argument of type 'ForwardRef' is not iterable

My current API is running perfectly with sqlmodel = "^0.0.8"

If someone wants to investigate the issue with me, I am willing to screenshare and show the project structure and try to make it to work; my Discord is filipemarch

But one thing I wonder, even if I were able to make this PR to work on my code base after countless changes, I don't think this is how library updates are supposed to work? Every time I type poetry update and more than 20 libraries update, I don't remember ever having to change anything in my code just to update any library.

This PR is certainly introducing breaking changes but I'm unable to tell what and why exactly, although I investigated a lot here I couldn't even make my project to run with it.

By the way I woud love to be using sqlalchemy 2.0 at this point, and it is very sad that sqlmodel seems to be abandoned since August 2022, not a single update.

@farahats9
Copy link
Contributor Author

@FilipeMarch you are right there is an issue in ForwardRef handling and I am trying to investigate it when I have some free time. The issue appears more when you have large project with many relations between tables so that's why some people report it working fine and some don't.

I feel this is a priority to fix over the mypy linting, I will do my best to figure it out but any help is appreciated.

@50Bytes-dev
Copy link

50Bytes-dev commented Jul 11, 2023

I had an error Mapped[" in raw_annotation but I was able to solve it. Here is an example of my models, works fine. Maybe it will help you.

# models/__init__.py
# need for alembic

from . import (
    company,
    location,
    client
)
# models/company.py

if TYPE_CHECKING:
    from models.location import Location
    from models.client import Client

class Company(BaseModel, CompanyBase, table=True):
    locations: List["Location"] = Relationship(back_populates="company")
    clients: List["Client"] = Relationship(back_populates="company")
# models/location.py

from models.company import Company

class Location(BaseModel, OrderedList, LocationBase, table=True):
    company_id: int = Field(default=None, foreign_key="company.id")
    company: Company = Relationship(back_populates="locations")
# models/client.py

from models.company import Company

class Client(ClientBase, BaseModel, table=True):
    company_id: int = Field(default=None, nullable=False, foreign_key="company.id")
    company: Company = Relationship(back_populates="clients")

@matanper
Copy link

Hey @farahats9,

I'm getting error when using sqlalchemy relationship:

class Hero(SQLModel, table=True):
    id: Optional[int] = Field(default=None, primary_key=True)

    powers: List["Power"] = Relationship(
        sa_relationship=relationship(back_populates='hero')
    )


class Power(SQLModel, table=True):
    id: Optional[int] = Field(default=None, primary_key=True)
    hero_id: int = Field(foreign_key="hero.id")
    hero: "Hero" = Relationship(back_populates="powers")
NameError: name 'Power' is not defined
NameError: Could not de-stringify annotation 'Power'

any idea why?

@AntonDeMeester
Copy link
Contributor

AntonDeMeester commented Jul 24, 2023

@matanper I think the forward Relationship (with the Foreign Key) needs to be a proper class, and not a type string. Can you try this?

class Hero(SQLModel, table=True):
    id: Optional[int] = Field(default=None, primary_key=True)

    powers: List["Power"] = Relationship(
        sa_relationship=relationship(back_populates='hero')
    )


class Power(SQLModel, table=True):
    id: Optional[int] = Field(default=None, primary_key=True)
    hero_id: int = Field(foreign_key="hero.id")
    hero: Hero = Relationship(back_populates="powers") # <== No quotes around Hero

@peterlandry
Copy link
Contributor

I've looked at this PR a bit to try cleaning up the linting errors. I've reduced most of them, but execute, get, and query are a little complicated because of the overloads. One thing I noticed in the process though is that sqlalchemy 2.0 has enough typing info to break mypy in query code. In VS code, given a sqlalchemy ORM model, if I do:

q = User.name == "spongebob"

q becomes an instance of ColumnElement[bool], while if User is a SQLModel, q goes straight to a bool as far as mypy is concerned.

This shows up as an error, but is correct at runtime. At runtime, we see an InstrumentedAttribute for the attribute, but mypy thinks it's a string. I'm going to look further at that, and in the meantime submit a pull request here that fixes the majority of the linting errors.

@honglei
Copy link

honglei commented Jul 28, 2023

Environment:

pydantic version: 1.10.11
python version: 3.11.4 (main, Jun 27 2023, 15:03:25) [GCC 8.3.0]
platform: Linux-4.19.0-amd64-desktop-x86_64-with-glibc2.28
optional deps. installed: ['devtools', 'email-validator', 'typing-extensions']
sqlalchemy: 2.0.19

pytest errors:

================================================================================================================================================================================================================== short test summary info ==================================================================================================================================================================================================================
FAILED tests/test_tutorial/test_create_db_and_table/test_tutorial001.py::test_create_db_and_table - assert 'BEGIN' in "No module named 'docs_src'\n"
FAILED tests/test_tutorial/test_fastapi/test_delete/test_tutorial001.py::test_tutorial - AssertionError: assert {'components'...or'}}, ...}}}} == {'components'...or'}}, ...}}}}
FAILED tests/test_tutorial/test_fastapi/test_limit_and_offset/test_tutorial001.py::test_tutorial - AssertionError: assert {'components'...Read Hero'}}}} == {'components'...Read Hero'}}}}
FAILED tests/test_tutorial/test_fastapi/test_multiple_models/test_tutorial001.py::test_tutorial - AssertionError: assert {'components'...eate Hero'}}}} == {'components'...eate Hero'}}}}
FAILED tests/test_tutorial/test_fastapi/test_multiple_models/test_tutorial002.py::test_tutorial - AssertionError: assert {'components'...eate Hero'}}}} == {'components'...eate Hero'}}}}
FAILED tests/test_tutorial/test_fastapi/test_read_one/test_tutorial001.py::test_tutorial - AssertionError: assert {'components'...Read Hero'}}}} == {'components'...Read Hero'}}}}
FAILED tests/test_tutorial/test_fastapi/test_relationships/test_tutorial001.py::test_tutorial - AssertionError: assert {'components'...or'}}, ...}}}} == {'components'...or'}}, ...}}}}
FAILED tests/test_tutorial/test_fastapi/test_response_model/test_tutorial001.py::test_tutorial - AssertionError: assert {'components'...eate Hero'}}}} == {'components'...eate Hero'}}}}
FAILED tests/test_tutorial/test_fastapi/test_session_with_dependency/test_tutorial001.py::test_tutorial - AssertionError: assert {'components'...or'}}, ...}}}} == {'components'...or'}}, ...}}}}
FAILED tests/test_tutorial/test_fastapi/test_simple_hero_api/test_tutorial001.py::test_tutorial - AssertionError: assert {'components'...eate Hero'}}}} == {'components'...eate Hero'}}}}
FAILED tests/test_tutorial/test_fastapi/test_teams/test_tutorial001.py::test_tutorial - AssertionError: assert {'components'...or'}}, ...}}}} == {'components'...or'}}, ...}}}}
FAILED tests/test_tutorial/test_fastapi/test_update/test_tutorial001.py::test_tutorial - AssertionError: assert {'components'...or'}}, ...}}}} == {'components'...or'}}, ...}}}}
===================================================================================================================================================================================

@tiangolo
Copy link
Member

TL;DR: Good news, this is available in the just released SQLModel 0.0.12! 🎉


Awesome, thanks a lot for all the work @farahats9! 🚀

And thanks everyone for the comments, reviews, etc. ☕

@farahats9 I continued the work here with several things on top on PR #700. I took your commits so you're co-author of that PR (and contributor to SQLModel, check your badge here next to your username, or in https://github.com/tiangolo/sqlmodel/graphs/contributors 😎 ).

I did a few extra things:

  • I reverted a few things, e.g. altering the docs to workaround for a previous lack of support for forward references.
  • Added support/fix for forward reference errors.
  • Removed several sections/modules that are no longer necessary, as were mainly to add type annotations to SQLAlchemy things, but now with SQLAlchemy's new proper types that's no longer necessary and I can reuse SQLAlchemy's things directly.
  • Updated all needed tests, e.g. to support the latest FastAPI.
  • Updated all the generated selects to use the new SQLAlchemy types.
  • Updated the classes and type annotations with the new SQLAlchemy types, with some overloads to make sure they are compatible with SQLModel. E.g. select().where().
  • Redefine all functions that would take column operations to accept the types used by SQLModel, e.g. or_(Hero.name == "Deadpond") as SQLAlchemy's version would require Hero.name to be a Mapped[str] instead of a str.
  • I explored a lot if it was possible/sensible to use and encourage the new Mapped[X], but sadly that breaks type annotations for creating new instances and dataclass_transform, as then that would expect to receive values with Mapped[X], e.g. Mapped[str] instead of str. And it would also add complications to the Pydantic handling. So, this was a lot of work mostly exploratory, but not much ended in the PR.

It is now available in SQLModel 0.0.12. 🎉

As this is now included in the repo and released, I'll now close this PR. Thanks! 🍰

@tiangolo tiangolo closed this Nov 18, 2023
@gdoumenc
Copy link

gdoumenc commented Nov 20, 2023

Hope it can help : I had the same issue because the relationsship was always defined and I didn' decalre it Optional :
origin_plan: t.Optional["Plan"] <== Good
instead of :
origin_plan: "Plan" <== Wrong

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.