📝 Add await in StreamingResponse code example to allow cancellation#14681
📝 Add await in StreamingResponse code example to allow cancellation#14681tiangolo merged 3 commits intofastapi:masterfrom
await in StreamingResponse code example to allow cancellation#14681Conversation
📝 Docs previewLast commit 7f49f35 at: https://51705672.fastapitiangolo.pages.dev Modified Pages |
This comment was marked as spam.
This comment was marked as spam.
Body: [`StreamingResponse` docs](https://fastapi.tiangolo.com/advanced/custom-response/?h=responses#streamingresponse) state: > Takes an async generator or a normal generator/iterator However based on Kludex/starlette#1776 (comment) `async` generetors need something `await`ed to work: ```py import asyncio from time import sleep from fastapi import FastAPI from fastapi.responses import StreamingResponse app = FastAPI() async def fake_video_streamer(): for i in range(10): sleep(0.1) yield b"some fake video bytes" # await asyncio.sleep(0) # <-- uncomment to fix example @app.get("/") async def main(): """ this is as per https://fastapi.tiangolo.com/advanced/custom-response/?h=responses#streamingresponse but does not to work (blocks for 1 sec, then returns all chunks at once): curl -sNo- localhost:8000 wget -qO- localhost:8000 """ return StreamingResponse(fake_video_streamer()) ``` - `uvicorn==0.40.0` - `fastapi==0.128.0` --- Comments --- User: casperdcl It seems based on Kludex/starlette#1776 (comment) that `await asyncio.sleep(0)` needs to be added to the documented example to work. User: mirza-mohibul-hasan Hi @casperdcl, thanks for raising this. I understand the issue and the root cause with async generators blocking without an await. If the fix isn’t already finalized, I’d be happy to work on this or help refine the docs/validate the example. Please let me know if that works for you. User: casperdcl fixed in fastapi#14681
Body: [`StreamingResponse` docs](https://fastapi.tiangolo.com/advanced/custom-response/?h=responses#streamingresponse) state: > Takes an async generator or a normal generator/iterator However based on Kludex/starlette#1776 (comment) `async` generetors need something `await`ed to work: ```py import asyncio from time import sleep from fastapi import FastAPI from fastapi.responses import StreamingResponse app = FastAPI() async def fake_video_streamer(): for i in range(10): sleep(0.1) yield b"some fake video bytes" # await asyncio.sleep(0) # <-- uncomment to fix example @app.get("/") async def main(): """ this is as per https://fastapi.tiangolo.com/advanced/custom-response/?h=responses#streamingresponse but does not to work (blocks for 1 sec, then returns all chunks at once): curl -sNo- localhost:8000 wget -qO- localhost:8000 """ return StreamingResponse(fake_video_streamer()) ``` - `uvicorn==0.40.0` - `fastapi==0.128.0` --- Comments --- User: casperdcl It seems based on Kludex/starlette#1776 (comment) that `await asyncio.sleep(0)` needs to be added to the documented example to work. User: mirza-mohibul-hasan Hi @casperdcl, thanks for raising this. I understand the issue and the root cause with async generators blocking without an await. If the fix isn’t already finalized, I’d be happy to work on this or help refine the docs/validate the example. Please let me know if that works for you. User: casperdcl fixed in fastapi#14681
Body: [`StreamingResponse` docs](https://fastapi.tiangolo.com/advanced/custom-response/?h=responses#streamingresponse) state: > Takes an async generator or a normal generator/iterator However based on Kludex/starlette#1776 (comment) `async` generetors need something `await`ed to work: ```py import asyncio from time import sleep from fastapi import FastAPI from fastapi.responses import StreamingResponse app = FastAPI() async def fake_video_streamer(): for i in range(10): sleep(0.1) yield b"some fake video bytes" # await asyncio.sleep(0) # <-- uncomment to fix example @app.get("/") async def main(): """ this is as per https://fastapi.tiangolo.com/advanced/custom-response/?h=responses#streamingresponse but does not to work (blocks for 1 sec, then returns all chunks at once): curl -sNo- localhost:8000 wget -qO- localhost:8000 """ return StreamingResponse(fake_video_streamer()) ``` - `uvicorn==0.40.0` - `fastapi==0.128.0` --- Comments --- User: casperdcl It seems based on Kludex/starlette#1776 (comment) that `await asyncio.sleep(0)` needs to be added to the documented example to work. User: mirza-mohibul-hasan Hi @casperdcl, thanks for raising this. I understand the issue and the root cause with async generators blocking without an await. If the fix isn’t already finalized, I’d be happy to work on this or help refine the docs/validate the example. Please let me know if that works for you. User: casperdcl fixed in fastapi#14681
|
This pull request has a merge conflict that needs to be resolved. |
YuriiMotov
left a comment
There was a problem hiding this comment.
@casperdcl, thanks!
I think we should add a note about this right after the code example.
Something like:
/// note
Note that we added `await asyncio.sleep(0)` inside the generator function.
This generator does not have any other `await` inside the loop. In asyncio, a task can only be cancelled when it reaches an `await`.
If there is no `await`, the generator can not be cancelled properly and may keep running even after cancellation is requested.
Adding `await asyncio.sleep(0)` gives the event loop a chance to handle cancellation.
///
What do you think?
|
rebased & added note :) |
|
You shouldn't update translations. Please, only update English version |
StreamingResponse code example
There was a problem hiding this comment.
LGTM!
@casperdcl, thank you!
Passing this to Sebastian for the final review
See also alternative wording: #14993
|
Hey @casperdcl, I reviewed and tested this PR locally. Testing Results:
The proposed fix correctly addresses the issue and makes the tutorial much more reliable for users. LGTM! |
StreamingResponse code exampleawait in StreamingResponse code example to allow cancellation
tiangolo
left a comment
There was a problem hiding this comment.
Thank you! 🚀
I also had that in mind when building https://fastapi.tiangolo.com/tutorial/stream-json-lines/ and https://fastapi.tiangolo.com/advanced/stream-data/ , so that it's handled automatically with that new style. (about to be released in the next hours).
Fixes #14680
asyncgenerators passed tostarlette.StreamingResponsemust contain anawaitcommand1 otherwise they blockFootnotes
https://github.com/Kludex/starlette/discussions/1776#discussioncomment-3207518 ↩