Revamp docs on Client instances #836
Merged
Conversation
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
yeraydiazdiaz
approved these changes
Feb 29, 2020
There was a problem hiding this comment.
This is great, thanks @florimondmanca!
I think it all reads quite well just left a comment to improve the conciseness of a section. 👍
docs/advanced.md
Outdated
|
|
||
| Indeed, when using the top-level API, an HTTP connection has to be opened and closed _for every single request_. | ||
|
|
||
| So, if you are making 50 HTTP/1.1 requests per second to a host using the top-level API, then HTTPX needs to open and shutdown 50 connections per second. This is a lot of work! It can quickly become expensive, and slow down your programs. Reasons include higher latency due to handshaking, higher CPU usage due to repeated TLS handshakes, higher network congestion due to a higher number of TCP connections, etc. |
There was a problem hiding this comment.
I would use this paragraph in place of the above benefits and maybe use bulletpoints in the reasons.
There was a problem hiding this comment.
Yes, the bullets and the "Reasons include" here do essentially repeat the same thing, so +1 to keeping just one of them, and using bullets.
There was a problem hiding this comment.
Thanks! I gave this section a different wording altogether to reduce the repetition. Let me know what you think!
hugovk
reviewed
Mar 1, 2020
Co-Authored-By: Hugo van Kemenade <hugovk@users.noreply.github.com>
hugovk
reviewed
Mar 1, 2020
Co-Authored-By: Hugo van Kemenade <hugovk@users.noreply.github.com>
|
Merging now, thanks both for the reviews! 👍 |
|
Ace, great work! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR updates the documentation on
Clientinstances:.close()-style usage..build_request()to more generally detail how to use explicitRequestinstances (can be linked to in Add docs on request instances to Requests compatibility guide #823 once this is merged).