-
-
Notifications
You must be signed in to change notification settings - Fork 354
[18.0] [FIX] fastapi: support both starlette < and >= 0.48 #572
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
Conversation
|
Hi @lmignon, |
1661881 to
6bc3764
Compare
lmignon
left a comment
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.
@ivantodorovich IMO We should revert the change related to status.HTTP_422_UNPROCESSABLE_ENTITY to support the same range of starlette version as in fastapi "starlette>=0.40.0,<0.50.0". The depreciation has been introduced in starlette 0.48....
|
Hi @lmignon ! What I don't like about that is that we will hit this issue in the future, at some point. Today it's a deprecation warning, but soon will be an exception. But I see your point wanting to support older versions, too.. so perhaps we could do the same That is, replace the use of these constants with their simple integer value (e.g.: I'll update this PR with a new proposal |
Starting from `starlette >= 0.48`, the `status.HTTP_422_UNPROCESSABLE_ENTITY` emits a deprecation warning, as it has been replaced by `status.HTTP_422_UNPROCESSABLE_CONTENT`. To be compatible with past and future versions, we simply use the integer value `422`, aligning also with the updated `fastapi` documentation: See: - fastapi/fastapi#14077
6bc3764 to
4c44017
Compare
|
@lmignon when you have a moment, please check the new proposal 🙏🏻 |
|
Thank you @ivantodorovich. This fix is pragmatic and solve the issue without introducing additional constrain. |
|
/ocabot merge patch |
|
Hey, thanks for contributing! Proceeding to merge this for you. |
|
Congratulations, your PR was merged at 62b5c7e. Thanks a lot for contributing to OCA. ❤️ |
Recent merged changes in a5bae2b changed
status.HTTP_422_UNPROCESSABLE_ENTITYtostatus.HTTP_422_UNPROCESSABLE_CONTENTin order to supress a deprecation warning emitted by thestarlette >= 0.48library.However, older versions of
starlettewill simply break with this change.For this reason, we bump the minimum required version offastapito 0.120.0, which consumes newer versions ofstarlettewhere this is not an issue.To be compatible with past and future versions, we simply use the integer value
422, aligning also with the updatedfastapidocumentation:See: