You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
How about provide non-contextmanager api for stream? #830
But, because the call of r.aiter_raw is after the call of stream context manager's __aexit__, the code above will raise a ResponseClosed exception.
I don't learn more about the design of httpx and starlette, and I think the simple way is providing non-contextmanager api for stream, just like AsyncClient. It can prevent we use dunder method directly:
If anything I’d probably say that it indicates Starlette request/response views are awkward for some streaming cases, rather than that we should change httpx.
^ It might be a good idea to document that pattern (which is also valid in the sync case). I think we already mentioned in the past expanding our docs to give .send() some light anyway, so…
What I can infer from @MisLink is that the dunder __exit__ method is called as soon as I return my response object; this marks the stream as is_closed (checked in models.py, in theiter_raw(self)) which raises a httpx.exception.ResponseClose and causes for the request to fail. For my use case I'm not using any of the async APIs.
I agree that maybe there should be a non-context manager API for streaming.
@triztian Essentially the « manual » mode with .send()is the non-context managed streaming usage.
You can adapt my async snippet from above to your sync use case: use a Client, use .send(), drop any async/await annotations, make sure to plug into Flask’s callback system to register response.close() (otherwise you’ll end up with dangling connections! This is the original motivation for encouraging context managed usage) and you should be good.
I was able to create a working solution by using the client API, thanks to @florimondmanca for mentioning it, however I think had he not mentioned it I would've spent a lot of time looking for an answer:
I think it would be really helpful to include this solution as part of the documentation in the Streaming Responses section, considering that most people that move from requests to httpx will find my first initial working solution and then land on the following page: Streaming Responses
I also struggled a bit with the Developer Interface: Client documentation, it includes the following example:
However the client.get method does not support the stream argument, yet requests.get does support it, while 100% equal APIs are not to be expected it can be a little confusing for newcomers to httpx (like myself)
Happy to contribute these changes, a special section under the README documentation that aids in requests migration from common patterns would be helpful.
Reacted by Florimond Manca, Chris Morgan and Qier LU
@tomchristie@florimondmanca thanks for your advices! send() api is good enough, using plain asgi protocol is also cute. And I think maybe somewhere could remind people content in StreamingResponse is a awaitable object even in sync call.
I confused httpx and starlette... don't mind.
Opened #840 with a docs update. It uses a different style (based on ExitStack/AsyncExitStack) than reaching into the .send() API so that .stream() remains the sole recommended entrypoint to streaming responses. Free to leave comments there!
It uses a different style (based on ExitStack/AsyncExitStack) than reaching into the .send()
I think documenting out build_request and send ar eprobably the right option here. I still want to stear folks away from this wherever possible, because it's a really good way of failing to close streaming response (vs. the context managed case, which protects you from that.)
The idea would be to drop the "get the response on enter idea", and instead do this:
Add context manager + async context manager methods to Response. __enter__ and __aenter__ are blank, and __exit__/__aexit__ call .close() / .aclose().
Make Client.stream() / await AsyncClient.stream() return a Response instance (instead of a context manager that we'd have to __enter__() to get the response object).
Non-context managed usage would then look like this:
Which seems like a rather natural transition form the context-managed usage (drop the async with and make sure to aclose the response).
Obviously this would be a breaking change (users need to add the extra await), but IMO one we might want to make before 1.0 lands.
For background, originally we discussed "responses as context managers" in #393. This was kept open as relevant for a while, and then considered "obsoleted" by the .stream() API (#600).
Activity
lovelydinosaur commentedon Feb 27, 2020
If anything I’d probably say that it indicates Starlette request/response views are awkward for some streaming cases, rather than that we should change httpx.
You can use httpx.stream and starlette’s StreamingResponse inside a plain ASGI app, which might be useful, eg... https://gist.github.com/tomchristie/5765e10a90a41c7e57470e2dc700f9db
florimondmanca commentedon Feb 28, 2020
One other approach would be to skip the
.stream()helper in your case and enter manual mode with.send():The behavior is the same than with the
__aenter__()/__aexit__()pair, but arguably easier on the eyes. :-)(
.send(stream=...)is public API — see theClientdeveloper interface.)florimondmanca commentedon Feb 28, 2020
^ It might be a good idea to document that pattern (which is also valid in the sync case). I think we already mentioned in the past expanding our docs to give
.send()some light anyway, so…triztian commentedon Feb 28, 2020
I think I'm running into the same issue, essentially I'm trying to replicate this usage pattern from requests, this works successfully:
And this is my
httpxversion, it always fails withhttpx.exception.ResponseClosedWhat I can infer from @MisLink is that the dunder
__exit__method is called as soon as I return my response object; this marks the stream asis_closed(checked in models.py, in theiter_raw(self)) which raises ahttpx.exception.ResponseCloseand causes for the request to fail. For my use case I'm not using any of theasyncAPIs.I agree that maybe there should be a non-context manager API for streaming.
florimondmanca commentedon Feb 28, 2020
@triztian Essentially the « manual » mode with
.send()is the non-context managed streaming usage.You can adapt my async snippet from above to your sync use case: use a
Client, use.send(), drop any async/await annotations, make sure to plug into Flask’s callback system to registerresponse.close()(otherwise you’ll end up with dangling connections! This is the original motivation for encouraging context managed usage) and you should be good.Let me know if you’re able to make it work. :)
triztian commentedon Feb 28, 2020
I was able to create a working solution by using the client API, thanks to @florimondmanca for mentioning it, however I think had he not mentioned it I would've spent a lot of time looking for an answer:
I think it would be really helpful to include this solution as part of the documentation in the
Streaming Responsessection, considering that most people that move fromrequeststohttpxwill find my first initial working solution and then land on the following page: Streaming ResponsesI also struggled a bit with the Developer Interface: Client documentation, it includes the following example:
However the
client.getmethod does not support thestreamargument, yetrequests.getdoes support it, while 100% equal APIs are not to be expected it can be a little confusing for newcomers tohttpx(like myself)Happy to contribute these changes, a special section under the README documentation that aids in
requestsmigration from common patterns would be helpful.MisLink commentedon Feb 28, 2020
@tomchristie @florimondmanca thanks for your advices!
send()api is good enough, using plain asgi protocol is also cute.And I think maybe somewhere could remind peoplecontentinStreamingResponseis a awaitable object even in sync call.I confused httpx and starlette... don't mind.
florimondmanca commentedon Mar 1, 2020
@triztian
This is a deliberate design decision on our end -- see #588 for background. :-)
florimondmanca commentedon Mar 1, 2020
Opened #840 with a docs update. It uses a different style (based on
ExitStack/AsyncExitStack) than reaching into the.send()API so that.stream()remains the sole recommended entrypoint to streaming responses. Free to leave comments there!lovelydinosaur commentedon Mar 2, 2020
I think documenting out
build_requestandsendar eprobably the right option here. I still want to stear folks away from this wherever possible, because it's a really good way of failing to close streaming response (vs. the context managed case, which protects you from that.)florimondmanca commentedon Jun 16, 2020
So, reading over this again, what do we think of this? …
Note the extra
awaitthere, similar to trio'sopen_file()API.The idea would be to drop the "get the response on enter idea", and instead do this:
Response.__enter__and__aenter__are blank, and__exit__/__aexit__call.close()/.aclose().Client.stream()/await AsyncClient.stream()return aResponseinstance (instead of a context manager that we'd have to__enter__()to get the response object).Non-context managed usage would then look like this:
Which seems like a rather natural transition form the context-managed usage (drop the
async withand make sure toaclosethe response).Obviously this would be a breaking change (users need to add the extra
await), but IMO one we might want to make before 1.0 lands.For background, originally we discussed "responses as context managers" in #393. This was kept open as relevant for a while, and then considered "obsoleted" by the
.stream()API (#600).