-
Notifications
You must be signed in to change notification settings - Fork 999
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 FastAPI to v0.70.0 #12836
Upgrade FastAPI to v0.70.0 #12836
Conversation
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.
I've given up on poetry for now - last time I tried to add a dependency and record how long it takes for it to solve the dependency graph, it took 10 hours before it died. So I just added the dependency manually. We may need to find a better way to do this, eventually, but for now, in my opinion, it's fine. (At least the risk of something conflicting - which we will find and fix sooner rather than later - outweighs the potential derailment of any PR that seeks to update a dependency 😄
b87ad99
to
af4ef0c
Compare
The failing tests seem unrelated:
So I guess there are no breaking changes after the upgrade 🎉 |
Ok, I've seen exceptions being logged in the currently failing Converters tests in the new pull requests due to FastAPI v0.70.0 is pinned to Starlette v0.16.0, so I guess we cannot just update to Starlette v0.17.0 yet. Should we downgrade FastAPI to v0.69.0 then or wait for a new release? |
The failing converter test has just been fixed by #12851 (already merged forward), but we probably still want to avoid the |
I will try to upgrade the FastAPI dependency to the latest version gradually as in the v0.70.0 release notes
Note: I'm going to update the dependencies manually because I can't make poetry work in a reasonable amount of time, so I will appreciate expert eyes on this... 😅
Reason
There are some bug fixes in both FastAPI and Starlette and we should try to keep in sync with the latest changes as much as we can at least until the new stack becomes the default.
Also, there is a fix for the previous breaking change in 0.65.2 that was forcing us to use JSON Content-type on all payloads, so after this, hopefully, there should be no need to explicitly set
json=True
in the API tests requests.How to test the changes?
License