-
-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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 Starlette from 0.18.0 to 0.19.0 #4488
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4488 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 531 531
Lines 13630 13629 -1
=========================================
- Hits 13630 13629 -1
Continue to review full report at Codecov.
|
📝 Docs preview for commit b19e848 at: https://61f2f482339a46310dd7a236--fastapi.netlify.app |
📝 Docs preview for commit 1b3342b at: https://61f42253a83c1522555c776e--fastapi.netlify.app |
📝 Docs preview for commit 09c70c4 at: https://61f6b9f3886122caa7fb096d--fastapi.netlify.app |
📝 Docs preview for commit 880dd0d at: https://61f8720b27a15533d4fa2da5--fastapi.netlify.app |
📝 Docs preview for commit c48a399 at: https://61f94879d85c46e5edab7f8e--fastapi.netlify.app |
📝 Docs preview for commit 7b519b8 at: https://61f9bcf8e9ce9131496180ee--fastapi.netlify.app |
fastapi/exceptions.py
Outdated
super().__init__(status_code=status_code, detail=detail) | ||
self.headers = headers | ||
|
||
from starlette.exceptions import HTTPException as HTTPException # noqa |
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.
Is the renaming of import, i.e., as HTTPException
needed at the last?
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.
from starlette.exceptions import HTTPException as HTTPException # noqa | |
from starlette.exceptions import HTTPException |
I suppose there's not reason for noqa
anymore.
This works great for me in the Fedora Linux RPM package. However, I still do have to ask
|
I need to update this PR. I'll do it today (I think). |
I'm going to just ignore the warning from @tiangolo This is the PR that deprecates it: encode/starlette#1504 I'm not sure if you want to import |
📝 Docs preview for commit d382aa5 at: https://62321787dc48cd00848f8cda--fastapi.netlify.app |
Still works for me, including the newly-ignored warning. Thanks! |
fastapi/exceptions.py
Outdated
super().__init__(status_code=status_code, detail=detail) | ||
self.headers = headers | ||
|
||
from starlette.exceptions import HTTPException as HTTPException # noqa |
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.
from starlette.exceptions import HTTPException as HTTPException # noqa | |
from starlette.exceptions import HTTPException |
I suppose there's not reason for noqa
anymore.
fastapi/routing.py
Outdated
@@ -126,7 +126,7 @@ async def serialize_response( | |||
if is_coroutine: | |||
value, errors_ = field.validate(response_content, {}, loc=("response",)) | |||
else: | |||
value, errors_ = await run_in_threadpool( | |||
value, errors_ = await run_in_threadpool( # type: ignore[misc] |
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.
How about adding proper type hints for these names?
def __init__( | ||
self, | ||
status_code: int, | ||
detail: Any = None, |
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.
There are several examples and places in the code base that expect detail
to be any JSON-able data, not only a string (as is with Starlette).
So, only for this little type hint we still need the custom HTTPException
class in FastAPI.
📝 Docs preview for commit fd46adb at: https://6279506becc1a10283b84487--fastapi.netlify.app |
Thanks for the work @Kludex! 🚀 🎉 This is now available in FastAPI version |
Co-authored-by: Sebastián Ramírez <[email protected]>
Set of breaking changes:
headers
parameter toHTTPException
encode/starlette#1435 (mypy)content
argument required toJSONResponse
encode/starlette#1431Starlette 0.19.0 is not released yet. Feel free to ignore this for now.