-
Notifications
You must be signed in to change notification settings - Fork 108
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
Turn transport.request() into a context manager #206
Closed
Closed
Changes from 20 commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
cc27a65
Turn transport.request() into a context manager
florimondmanca dcf015e
Fix Python 3.6 compatibility
florimondmanca 4530efe
Sync docs index snippets
florimondmanca 6be7698
Tweak call order of _response_closed
florimondmanca 3dfe78e
Merge branch 'master' into fm/request-context-manager
florimondmanca 35e006a
Lint
florimondmanca 793f285
Drop exitstack from connection_pool implementation
tomchristie aa99f4b
Drop ResponseStream.close in http11 implementation
tomchristie 02de08a
Drop ResponseStream.close in http2 implementation
tomchristie e7612a5
Merge master
tomchristie fdccfaf
Resolve typo in README
tomchristie 9d7885e
Ensure response_closed is called
tomchristie a92a82d
Drop close on bytestream interface
tomchristie 3699f46
Drop bytestream.close in docs API reference
tomchristie a6cafff
Merge branch 'master' into fm/request-context-manager
tomchristie 262464c
ByteStream -> Iterable[bytes]
tomchristie 9b0cdb0
Drop SyncByteStream, AsyncByteStream in favour of Iterable[bytes]
tomchristie f9563cf
Neater max_streams_semahore now that we have context-managed flow, ra…
tomchristie 4bacd4a
Merge branch 'master' into fm/request-context-manager
florimondmanca d7e9b44
Update new tests from master
florimondmanca 824d4ca
Coverage
florimondmanca 807e20b
Merge branch 'master' into fm/request-context-manager
florimondmanca File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I guess we just need to be careful that this drops
SyncByteStream
andAsyncByteStream
.We rely on these in a few places in HTTPX: https://github.com/encode/httpx/pull/1399/files#diff-f7cafd3bb44d8d834be724482dacf4ba57c93e7faa62bdce50fc05c00557e222R79
Some other implementations out there would also still be relying on these interfaces.
Should we perhaps keep some aliases in place until 1.0…?
… Or just delay merging these "request context manager" PRs until the next minor release cycle? (We're pinning HTTPCore to a minor in HTTPX anyway.) Yeah, thinking about it — I think that's probably the plan? :-)
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.
Yeah I don't think we're sufficiently confident yet about exactly where we're going with this interface,
so I reckon rolling the other stuff (where we are confident) and keeping this hanging a little longer.