flask-sqlalchemy icon indicating copy to clipboard operation
flask-sqlalchemy copied to clipboard

Migration to SQLAlchemy 2.0

Open tgross35 opened this issue 2 years ago • 10 comments

SQLAlchemy is moving toward a new system, detailed here: https://docs.sqlalchemy.org/en/14/changelog/migration_20.html. Flask-SQLAlchemy should also move in that direction.

The main difference is that session.query(Model) is being replaced, essentially with, session.execute(select(Model)). The old Session.query() will still be around, but is moved to legacy essentially dropped from maintenance / feature improvement.

My suggestion is to add a Model.select() method that adheres to this new format. This should be pretty straightforward and possibly already in the works (I didn't see anything though).

There are likely other ways that FSA could start moving to SQLA 2.0, but the select method is probably a top priority.

tgross35 avatar Oct 07 '21 17:10 tgross35

Happy to review a PR

davidism avatar Oct 07 '21 17:10 davidism

TBH the "SA 2.0" way looks more awkward for most simple usecases. I think Model.query should stay - without deprecation warnings.

That said, a nice and dev-friendly way of supporting the 2.0 way is certainly a good idea.

ThiefMaster avatar Oct 07 '21 17:10 ThiefMaster

I don't believe the query property is deprecated. I know I looked at this before and didn't see anything in particular that Flask-SQLAlchemy would need to change.

davidism avatar Oct 07 '21 18:10 davidism

Happy to make a PR if I get the time lol. Lacking the background in FSA as of now.

Anyway, David is right that it's not technically depreciated. It will still be around for possibly all of eternity, so FSA will continue to work as-is. But, query also won't get any new features/improvements, whereas select will. Agreed that SA 2.0 looks more awkward but it does seem to align better to the behind the scenes SQL, and give better hints as to what the return type might be.

It will probably eventually also make sense to update the FSA docs to encourage select whenever SA starts to make the official switch to 2.0. That seems like a way off for now though.

tgross35 avatar Oct 07 '21 18:10 tgross35

Reading through this now https://docs.sqlalchemy.org/en/14/changelog/migration_20.html#overview

Looks like there's more to this migration than I thought, the .select method is only the tip of the iceberg. Could possibly be some behind the scenes deprecated uses that should be resolved. These can be found by setting SQLALCHEMY_WARN_20 =1 env variable. That's easy enough to do and step through the recommended changes, which should be doable without changing the interface.

The main changelist to ORM syntax is here: https://docs.sqlalchemy.org/en/14/changelog/migration_20.html#migration-orm-usage. I kind of agree that the new implementation looks a lot more awkward with a little less abstraction from plain SQL.

Something interesting to note is that the new syntax breaks out the server-side select/filter/limits, and the "secondary" processes that convert it to results (currently operations like .first(), .all(), etc.). As an example:

# old sqla
session.query(User).filter_by(name='some user').first()

# new sqla
session.execute(
  select(User).
  filter_by(name="some user").
  limit(1)
).scalars().first()

Everything primary is wrapped in execute, everything secondary comes after. We could flatten it in FSA:

# possible "flat" fsa implementation, basically an alias of query
User.select.filter_by(name="some user").limit(1).scalars().first()

It would be nice to maintain the "hierarchy" that's introduced with this select statement, but it's not as pretty.

# 1: possible split fsa implementation, downside is namespace clutter on User
from flask_sqlalchemy import execute as ex
ex(User.select.filter_by(name="some user")).scalars().all()

# 2: return sqlalchemy.select as db.select (or import select directly without db wrapper), add execute
# method to User as wrapper for its session.execute
User.execute(db.select.filter_by(name="some user")).scalars().all()

# 3: possible implementation just using db.ex as a shortcut for session.execute
db.ex(
  db.select(User).        # or User.select, or select(User) if select imported from SA
  filter_by(name="some user").
  limit(1)
).scalars().first()

Honestly, none of this looks super clean to me, curious if anyone has any better ideas. db.session.execute(select(User)...)... should work in any case.

Also, side note - Flask and SQLA should probably be in requirements/dev.txt, following CONTRIBUTING.rst won't result in a workable test environment unless those are installed manually

tgross35 avatar Oct 21 '21 04:10 tgross35

All the stuff you're pointing to isn't affected by Flask-SQLAlchemy. You can do db.session.execute already, and since query_property isn't deprecated you can do all that still too.

I guess I'd consider a Model.select property, but honestly I'd be fine leaving the current query property and just letting people use more "plain" SQLAlchemy with db.session.execute(select(Model))) for the new style.

Also, side note - Flask and SQLA should probably be in requirements/dev.txt, following CONTRIBUTING.rst won't result in a workable test environment unless those are installed manually

That's what the pip install -e . part of the instruction is for.

davidism avatar Oct 21 '21 12:10 davidism

Alright, no changes to Model but I'll do a quick test with the depreciation warning just to see.

pip install -e . didn't catch those two for me, but I guess that might be a problem on my side somehow.

tgross35 avatar Oct 21 '21 14:10 tgross35

I just want to throw out a big advantage of 2.0 will be:

new constructs that will allow for construction of declarative mappings in place which will support proper typing directly, without the need for plugins

source

It would be amazing if flask-sqlalchemy could support that, so typing information flows through correctly.

davetapley avatar Jun 02 '22 21:06 davetapley

There is nothing extra to do here to support that

davidism avatar Jun 03 '22 02:06 davidism

Our test suite fails if we run it with SQLALCHEMY_WARN_20=1:

E sqlalchemy.exc.LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)

Not necessarily fully a flask-sqlalchemy issue, but it just so happens that we're using the soft delete Miguel wrote: https://blog.miguelgrinberg.com/post/implementing-the-soft-delete-pattern-with-flask-and-sqlalchemy

I think it breaks get_or_404() which perhaps could be fixed on flask-sqlalchemy's side using the new construct? More generally, we certainly could make use of an updated version of that article, but I'm still investigating which other 2.0 things might be incompatible with flask-sqlalchemy.

Cheaterman avatar Jun 30 '22 16:06 Cheaterman

#1087 bumps the minimum version of SQLAlchemy to 1.4. Additionally, it's possible to set future mode by passing engine_options={"future": True}, session_options={"future": True}. I've mentioned in multiple places in the documentation that the query interface is legacy in SQLAlchemy 2.0. After discussing with SQLAlchemy maintainers, we think it makes sense to add the query methods directly on db to support select statements. Beyond these changes, I'm happy to consider specific things, but I'll ask that specific issues are opened for each.

davidism avatar Sep 18 '22 17:09 davidism