-
-
Notifications
You must be signed in to change notification settings - Fork 6.3k
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
Upgrade databases and SQLAlchemy #5799
Conversation
📝 Docs preview for commit 574b0a2 at: https://639f3fe2d8175d240d2d6490--fastapi.netlify.app |
574b0a2
to
bd2295f
Compare
📝 Docs preview for commit bd2295f at: https://63b9f42d42bbfe7af491a2cf--fastapi.netlify.app |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checked out this PR, installed updates, and ran tests.
Test failing:
FAILED tests/test_tutorial/test_async_sql_databases/test_tutorial001.py::test_create_read - sqlalchemy.exc.RemovedIn20Warning: Deprecated API features detected! These feature(s) are not compatib...
sqlalchemy.exc.RemovedIn20Warning: Deprecated API features detected! These feature(s) are not compatible with SQLAlchemy 2.0. To prevent incompatible upgrades prior to updating applications, ensure requirements files are pinned to "sqlalchemy<2.0". Set environment variable SQLALCHEMY_WARN_20=1 to show all deprecation warnings. Set environment variable SQLALCHEMY_SILENCE_UBER_WARNING=1 to silence this message. (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
Thanks for looking at it. Those warnings were added in sqlalchemy 1.4.46, which was released after I opened this PR. I see three possible courses of action:
I’ll take a closer look at this and see if there is an obvious way to do option 3. |
Here’s the full output:
Unfortunately, this doesn’t carry any information about which deprecated API feature was used, and it’s hard to even be sure where the deprecated usage is happening. I tried setting
That points at an incompatibility in FastAPI, and it is something specific enough I can reasonably filter it out. |
bd2295f
to
2deb233
Compare
Added |
📝 Docs preview for commit 2deb233 at: https://63c5eb1e4a9d912fcb4f5299--fastapi.netlify.app |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After warning suppressed, tests are passing.
2deb233
to
656ce50
Compare
📝 Docs preview for commit 656ce50 at: https://64038cb82bd72c60f5c3e0c2--fastapi.netlify.app |
656ce50
to
262fa51
Compare
Rebased on |
📝 Docs preview for commit 262fa51 at: https://640c9640611f6123320901d1--fastapi.netlify.app |
262fa51
to
c4a4592
Compare
Rebased on |
📝 Docs preview for commit c4a4592 at: https://64722e9cae27a515156fe6e0--fastapi.netlify.app |
c4a4592
to
3017463
Compare
Rebased on |
📝 Docs preview for commit 3017463 at: https://64930f371649720e5435aa03--fastapi.netlify.app |
7c97a7d
to
2be88d5
Compare
c08d33d
to
7bc2ffb
Compare
Rebased again. |
@tiangolo, any interest in merging this? I’ve been carrying it as a downstream patch in Fedora Linux for quite a while, and it seems like a straighforward improvement that would allow CI testing with current SQLAlchemy releases in the 1.4.x series. |
7bc2ffb
to
feab74c
Compare
feab74c
to
f212102
Compare
Can you clarify. Is SqlAlchemy 2.0 not supported yet? Or is that just a limitation of testing? |
These are only test dependencies. The actual FastAPI package doesn’t import This PR predates the SQLAlchemy 2.0.0 release, so updating to Still, even if it wouldn’t allow testing with SQLAlchemy 2.0, this PR would at least improve the status quo. |
f212102
to
6f11a91
Compare
Rebased on |
It looks like this is no longer necessary in the current version of FastAPI, so I dropped the second commit. For posterity, since I’m force-pushing again, that commit looked like:
|
6f11a91
to
6bccc4d
Compare
Hmm, the warning doesn’t show up in local testing, but still appears in the |
260844a
to
03cb89b
Compare
Rebased on |
03cb89b
to
dca742e
Compare
Rebased on |
📝 Docs preview for commit dca742e at: https://e0ca5caf.fastapitiangolo.pages.dev |
Now upgraded to allow current releases of Tests are still passing for me locally; we will see if the CI agrees. |
Trying to adjust the warning configuration so that this still works with Pydantic v1… |
84ee2f1
to
229748d
Compare
📝 Docs preview for commit 229748d at: https://b684d8ee.fastapitiangolo.pages.dev |
229748d
to
25fe108
Compare
📝 Docs preview for commit 25fe108 at: https://63296ba9.fastapitiangolo.pages.dev |
Now I’m seeing issues like:
and
with Pydantic v1. I am not sure what to do about that. Suggestions welcome. This is still useful as a downstream patch in Fedora, where we have Pydantic v2 only and would like to test with current versions of everything. |
25fe108
to
5c88dac
Compare
📝 Docs preview for commit 5c88dac at: https://71d46b8f.fastapitiangolo.pages.dev |
4f8c92e
to
915e182
Compare
Incompatibilities with recent SQLAlchemy versions are fixed in databases 0.7.0. Allow databases 0.9.x and SQLAlchemy 2.x for testing. Ignore SQLAlchemy RemovedIn20Warning from FastAPI and Pydantic
915e182
to
89c3714
Compare
This PR has languished long enough, and since it occurs to me that I no longer really need it downstream in Fedora – I no longer use |
Incompatibilities with recent SQLAlchemy versions are fixed in databases 0.7.0. By requiring
databases[sqlite] >=0.7.0,<0.8.0
for testing, we can stop pinning an old SQLAlchemy version, instead requiringsqlalchemy >=1.4.42,<1.5
.Fixes #5556.