Skip to content
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

Context Managed Transports #1145

Closed
tomchristie opened this issue Aug 7, 2020 · 1 comment · Fixed by #1218
Closed

Context Managed Transports #1145

tomchristie opened this issue Aug 7, 2020 · 1 comment · Fixed by #1218
Labels
enhancement New feature or request
Milestone

Comments

@tomchristie
Copy link
Member

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... #998 (comment)

@tomchristie tomchristie added the enhancement New feature or request label Aug 7, 2020
@tomchristie tomchristie added this to the v1.0 milestone Aug 7, 2020
@florimondmanca
Copy link
Member

florimondmanca commented Aug 8, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants