Remove fastapi-users-db-sqlmodel dependency

[tcompa]

Context: this question came up while working (with @mfranzon) on a pydantic/sqlmodel/sqlalchemy issue which is not yet clear. We don't know if the two issues are related, but in general this one here has to be considered at some point.

--

We depend on fastapi-users-db-sqlmodel, that currently (as of their 0.2.0 version) forces sqlalchemy version to be <=1.4.35. The reason is that 1.4.36 introduced a widespread issue, including in sqlmodel - see release notes and several issues:

• SQLAlchemy version 1.4.36 breaks SQLModel relationships fastapi/sqlmodel#315
• Last version(1.4.36) broke SQLModel relationship. Any idea why ? sqlalchemy/sqlalchemy#7972 (reply in thread)
• 🐛 Fix SQLAlchemy version 1.4.36 breaks SQLModel relationships (#315) fastapi/sqlmodel#461

sqlmodel 0.0.7 partly fixed the issue on their side (but notice another PR is still open), and as of sqlmodel 0.0.8 (released in August 2022) their sqlalchemy constraint is ">=1.4.17,<=1.4.41".

In fastapi-users-db-sqlmodel there are several automatic dependabot PRs to relax the constraint, but little activity.

We should verify if this package actively mantained, and otherwise we should find some way to replace it.

Note that no other dependency of ours currently requires sqlalchemy<=1.4.35:

$ poetry show sqlalchemy

 name         : sqlalchemy                   
 version      : 1.4.35                       
 description  : Database Abstraction Library 

dependencies
 - greenlet !=0.4.17

required by
 - alembic >=1.3.0
 - fastapi-users-db-sqlmodel >=1.4,<1.4.36
 - sqlalchemy-utils >=1.3
 - sqlmodel >=1.4.17,<=1.4.41

[tcompa]

For the record, let's keep in mind that https://github.com/fastapi-users/fastapi-users-db-sqlalchemy appears as well maintained.

[tcompa]

The three classes that we are using from this package are

• SQLModelUserDatabaseAsync
• SQLModelBaseUserDB
• SQLModelBaseOAuthAccount

If we proceed with a switch from UUID4 to autoincremental integer IDs (ref #560), then SQLModelBaseUserDB and SQLModelBaseOAuthAccount become irrelevant.

As per SQLModelUserDatabaseAsync, right now I find it is used eight times across github, and also see comments like these ones

Bear in mind though that we didn't officially release this DB adapter; as they are some problems with SQLModel regarding UUID (fastapi/sqlmodel#25). So you'll probably run into issues.
..
There is one that should work, but don't plan to officially support it for now: https://github.com/fastapi-users/fastapi-users-db-sqlmodel

I think that we should quickly move on and explore the possibility of fully removing this dependency. If it impossible to replace SQLModelUserDatabaseAsync, then let's also consider that it is released under MIT license and we could include that definition in our code base.

[caniko]

Hello, would you consider supporting sqlmodel for fastapi-users?

[tcompa]

Hello, would you consider supporting sqlmodel for fastapi-users?

Hi there,
If you're asking whether we'd like to maintain https://github.com/fastapi-users/fastapi-users-db-sqlmodel or similar packages, I think we don't. We'll likely drop this dependency soon, and at some point we'll also need to review our use of sqlmodel in this project (as opposed to sqlalchemy v2).

[jluethi]

We being the Fractal developers, not the fastapi-users developers. You may want to check with them directly for your question @caniko , because we just use that library :)

[caniko]

I considered the same @tcompa, thanks

[tcompa]

This is now ready as part of #660. The choices we made are:

1. We merged the relevant part of SQLModelBaseUserDB into our own UserOAuth
2. We kept SQLModelBaseOAuthAccount as is (bundled in fractal-server with attribution, under MIT license), since it is a very transparent model with a few simple (string) attributes.
3. We kept SQLModelUserDatabaseAsync as is (bundled in fractal-server with attribution, under MIT license), since we could only re-implement it in the exact same way. If (unlikely) fastapi-users is updated in a breaking way (e.g. requiring an additional get_by_xxx method), then the current adapter won't work and some users endpoints may be break. This is mitigated by the fact that we already test all relevant fastapi-users endpoints (see test_security.py), and notably:
   • whoami
   • register
   • userlist
   • get user
   • delete user
   • patch user
   • create (super)user

Side note: if we ever move away from SQLModel, towards sqlalchemy v2, then we could revert the current change and easily switch to the (better maintained) corresponding adapter: https://github.com/fastapi-users/fastapi-users-db-sqlalchemy

[tcompa]

Closed with #660