Upgrade to SQLAlchemy 2

[pmdevita]

#1177

• Fix issues upstream with Databases Fix some column types being parsed twice encode/databases#582
• Update dependencies on Databases and SQLAlchemy
• Update Ormar for SQLAlchemy 2 changes
  • select() now take multiple params rather than a collection
  • SQLAlchemy now compiles some statements differently, but logically equivalent

[codspeed-hq]

CodSpeed Performance Report

Merging #1269 will not alter performance

_{Comparing pmdevita:feature/sqlalchemy2 (a7841b9) with master (62da098)¹}

Summary

✅ 84 untouched benchmarks

Footnotes

1. No successful run was found on master (a783e44) during the generation of this report, so 62da098 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

[collerek]

I know that we use sqlalchemy only in the core part but to be honest it's much fewer changes than I expected in this codebase for v2.0 support😅
Also appreciate you took it and solved it with databases too 🚀
Great job! 🎉
One test is failing now on parenthesis comparison (I think the one you changed).

[pmdevita]

Yep, I haven't updated Poetry yet since I'm still waiting on databases to make a release. I can update it to point at their GitHub for now if you just want to see it pass.

Databases has now also dropped support for 3.7, should I do that as well as part of this PR?

It was a lot less changes than I was expecting too lol, I guess databases just bears most of the work for this.

[collerek]

I can wait for the release, they just did 0.9.0 bit not sure if your change was included, guess so as it was merged

[pmdevita]

Oh yeah 0.9 should be good, I'll finish this up soon then

[pmdevita]

@collerek We need to remove 3.7 support in order to merge this, should I make another PR to do that? We might want to add 3.12 too while we're at it

[collerek]

You can remove it here, it's removed in pydantic v2 pr already

[collerek]

BTW it seems like a significant performance degradation over the previous version, we need to check if caching is working as described here: https://docs.sqlalchemy.org/en/20/faq/performance.html

[gmekicaxcient]

How can I and/or others help? I just don't want to step on anyone's toes.

[pmdevita]

Sorry, I'm going to try to come back around to this soon. I think there's a single PostgreSQL test that is failing still and there are the performance problems mentioned by collerek earlier. If you want, you can fork my branch and send a PR for me to merge into this one. Let me know if you want to try and tackle one of those, thanks for the help!

[gmekicaxcient]

I'll take a look, I still don't know if I have enough expertise to actually do something useful. If I find something, you'll get PR, of course.

[mekanix]

@pmdevita I was at liberty to cherry-pick your changes onto master branch and create feature/sqlalchemy2. On my machine testing signals fails, but I'll leave that for later. I'll try to find what's going on with performance, first.

[mekanix]

I did get [no key] in the output of benchmarks, but I never got sqlalchemy.ext.SAWarning nor did I find that TypeDecorator is used in ormar or databases. Any tips on where to look next, as this is caching problem. I am uploading output of pytest -s benchmarks just for reference.

[pmdevita]

I don't just want to sell out on the databases project... but its not getting a lot of support these days, and SQLAlchemy 2.0 does support async on its own now. I don't really know that much about Ormar's internals but is it worth cutting out the middle man and just working with SQLAlchemy directly?

[pmdevita]

Well I went and dug a bit myself, looks like we only use the databases library to execute SQLAlchemy expressions asynchronously in QuerySet, which is very possible now in SQLAlchemy 2. There would be some public API changes since we'd no longer use databases to create the DB connection, but it doesn't seem like it would be a huge hassle and we'd benefit by not having to worry about the state of the databases library. Do you have any thoughts @collerek?

[collerek]

Yeah I was thinking about ditching databases too as it has maintenance issues, we can check the effort to switch to Sqlalchemy completely also for query execution.

[mekanix]

I have some free time now, so I could help with something. When ormar migrated from pydantic1 to pydantic2 I restructured the tests and fixed some documentation. Maybe we can split/organize migration to sqlalchemy2, too?

[pmdevita]

Could you experiment with removing databases and just using SQLAlchemy directly? There are only a few places it gets called so I think on the surface it should be relatively trivial, but the way we configure database connections will probably change significantly

[dekoza]

Is there anything that I could help with to move this forward? Any luck with removing databases dependency? If not, maybe we could just move to sqla2 and then remove databases?

[pmdevita]

Finally circling back to this. It looks like the biggest obstacle to removing databases right now is transaction support, we relied entirely on databases for this prior and now we need to implement our own using SQLAlchemy. I'm going to make an attempt at this that's hopefully good enough to get tests going, most of the test suite relied on transactions.

I'm looking at Django as an example of how to present an API for transactions. The transaction will create a database connection and queries within the context will swap to using it. I'm using contextvars to implement this so there shouldn't be any async issues. Let me know if you have any ideas or philosophies on how to best do this too.

Also, databases is now archived as of this month RIP

[collerek]

Added support for 2.0 in #1470