Closed tomchristie closed 4 years ago
Hey,
Agreed we want clients to call into context manager dunder methods wherever appropriate.
~While looking over #998 I also think we have currently a problem re: .aclose()
vs __aexit__(*exc_info)
in clients (and perhaps transports?).~
~Right now the client calls .aclose()
in its __aexit__()
method, which calls into the .aclose()
of transports, but this is not correct. If an exception occurs, then we want it to propagate in the __aexit__()
of transports. Right now it won't, because we're calling .aclose()
on transports which does nothing. It's a bit hard to explain in words but I might submit a PR to demonstrate this point…~
(Edit^: this doesn't actually seem to be a problem for this particular case. Hmm…)
Also, as I think I've said already, I really don't think this issue applies much to ASGITransport
. The nursery object we need for ASGITransport
is request-level, not client-level (since you want the app to stop when the request is finished), so whether clients call into __aenter__()
/__aexit__()
is not really relevant.
We need to ensure that the Client
__enter__
/__exit__
and__aenter__
/__aexit__
methods also call into the transport dunder methods, for all installed transports/proxies.A good proof of having resolved the issue here would be streaming responses in the
ASGITransport
, which require nursery functionality in order to work properly. See notes here... https://github.com/encode/httpx/pull/998#issuecomment-653519294