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

Reintroduce sync API. #735

Merged
merged 14 commits into from
Jan 8, 2020
Merged

Reintroduce sync API. #735

merged 14 commits into from
Jan 8, 2020

Conversation

tomchristie
Copy link
Member

@tomchristie tomchristie commented Jan 7, 2020

Closes #572

  • Added Client.
  • Top-level API becomes sync.
  • Added URLLib3Dispatcher.
  • Wire up timeouts.
  • Wire up pool limits.
  • Catch and wrap urllib3 networking exceptions.
  • Add WSGIDispatcher.
  • Add urllib3 proxy dispatcher.

@tomchristie tomchristie added this to the 0.11.0 milestone Jan 7, 2020
@tomchristie tomchristie marked this pull request as ready for review January 7, 2020 16:21
@tomchristie
Copy link
Member Author

Figure we can probably mark this as ready for review here.

We probably still want to do a bit of work on wrapping exception classes, but that could be a follow-on PR.

  • The pool limits don't quite match up to what urllib3 uses, but it'll do.
  • write_timeout isn't supported.
  • I don't think we have any integration tests for proxy support, so I've not verified the proxy case properly yet.
  • Not sure that urllib3 supports the CONNECT_ONLY/TUNNEL_ONLY modes.

Copy link
Member

@florimondmanca florimondmanca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, I've looked at this for some time and played with it in the console, looks good! 🥇 Just a few minor comments.

Though I can't wait for us unasync'ing all this duplicated code in the client, maybe someday? 🤣

httpx/dispatch/wsgi.py Outdated Show resolved Hide resolved
httpx/client.py Outdated Show resolved Hide resolved
@tomchristie
Copy link
Member Author

Though I can't wait for us unasync'ing all this duplicated code in the client, maybe someday? 🤣

I think that one is a solid maybe, and could go either way. 😃

The only non-trivial chunk of stuff that unasync would deduplicate is https://github.com/encode/httpx/blob/sync/httpx/client.py#L1040-L1216 which is a fairly modest amount for us to maintain, so I could equally well see an unasync approach only ever applied to the dispatch layer. But open to either.

This was referenced Jan 8, 2020
@tomchristie tomchristie added enhancement New feature or request api change PRs that contain breaking public API changes labels Jan 8, 2020
@florimondmanca
Copy link
Member

In any case I think we'll start with the dispatch layer, if/when we switch away from urllib3. :-)

@tomchristie tomchristie merged commit ee37a76 into master Jan 8, 2020
@tomchristie tomchristie deleted the sync branch January 8, 2020 12:31
@tomchristie tomchristie mentioned this pull request Jan 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api change PRs that contain breaking public API changes enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Supporting Sync. Done right.
2 participants