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

Getting to 1.0 #715

Closed
tomchristie opened this issue Jan 2, 2020 · 4 comments
Closed

Getting to 1.0 #715

tomchristie opened this issue Jan 2, 2020 · 4 comments

Comments

@tomchristie
Copy link
Member

An informal issue here to talk about one route of getting to 1.0, that I think makes a bunch of sense...

  • Remove httpx.HTTPProxy from the public API, in favor of httpx.Proxy, which is an object that just holds the proxy config, rather than a Dispatcher instance. Prep for introducing SyncClient #713
  • Add a SyncClient implementation, initially requiring dispatch=... be passed, at this point we can introduce tests for the sync client.
  • Add WSGIDispatcher, and support SyncClient(app=...).
  • Add URLLib3ConnectionPool, and URLLib3ProxyManager implementations, and use those by default for the sync case. This gives us urllib3 backing for the sync case, and our own dispatch for the async case.
  • Switch the top-level API to the sync case.
  • Drop any remaining bits of API that we currently have warnings for.
  • Cut a 1.11 release.

At this point we're in great shape - we've got all our public API how we think we want it to look for 1.0, and we've got sync+async support.

(We won't have sync HTTP/2 support at that point, because we're backed by urllib3 there.)

We can then:

  • Use an unasync approach to provide a sync dispatch implementation.
  • Work on resolving any criticial issues that we think would block our users from wanting to switch away from the urllib3 dispatcher implementation.
  • Switch the sync dispatcher to us our built-in version, with a URLLib3Client provided as an alternate case.
  • Issue a 1.0 release.

What I think is compelling about this route is that it gets us to an "API complete" state as quickly as possible, while still allowing us to eventually switch to using a single unified dispatch implementation for the 1.0 release.

@tomchristie
Copy link
Member Author

Alrighty, I wanted to check that there's no obvious blockers to 1.11, so pressed on with a draft PR to demonstate... #716

Anyways, yay, I reckon we're just about there!

We may want to think about our review process, and address the changes bit by bit to make sure they get properly reviewed, but that PR demonstates how it'll look.

@florimondmanca
Copy link
Member

That’s cool! But let’s hold off onto #716 for a bit, maybe?

The approach taken here is to duplicate a bunch of code, and reintroduce the BaseClient construct we had before 0.8.

Is that the final situation we’d like to end up in? I’m not sure…

I’d really see unasync help us out a lot here. If we go that path, while it’s okay for us to duplicate code for now, I’m very much in favor of unasyncing the client for 1.0, and not after (if that was the strategy), for the sake of derisking and taking our time to actually achieve stability (beyond API stability).

As for urllib3, my first reaction to eventually dropping it in favor of our own unasync’ed dispatchers was “why bother?”, though the main argument in favor of that might be to guarantee consistent behavior across sync and async. But if we’re able to get that with urllib3, then fine, maybe?

Also, I’m not sure I get the idea of an Urllib3Client as an alternate case — would that be for backwards compatibility? Or really only to provide an alternative client implementation? Not sure we want to maintain two implementations and expose them for our users to make a choice?

@florimondmanca florimondmanca pinned this issue Jan 3, 2020
@tomchristie
Copy link
Member Author

Fair questions!...

So, there’s a big difference from before:

  • We’ve got unified Request and Response classes, no more gnarliness with BaseRequest, Request, AsyncRequest, BaseResponse, Response, AsyncResponse and passing around backend instances.
  • Also no more wierdness with async under the hood and sync on the surface, if you want to write a sync dispatcher, it’s actually a plain old sync dispatcher.
  • Our bridging approach was somewhat gross, and leading to a bunch of intractable bugs.

Granted, we’ve got a bit of replication here, but it’s tightly limited in scope, and pretty tractable. I’d always been expecting that any unasync would only be targeted at the dispatch implementations. We can’t really unasync the Client in quite the same way since there are different function signatures in cases, and slightly different implementations in others. (Eg init-ing the dispatcher and proxy instances)

Wrt. urllib3, I’m pretty happy on this. It’s not totally obvious if we would want to later switch it out to a native unasync’ed version or not. Doing so would give us more consistent sync/async behaviour and sync http/2 support, plus I think our native network handling is looking pretty sharp now, so my general thought at this point in time is that we probably would want to do so once we feel our async case is sufficiently mature to justify switching.

If we do switch our underlying sync implementation at a later point it’d still be super valuable to provide a urllib3 option (ie requests users that want to switch over to httpx but want to ensure that their http behaviour stays as close as possible)

@tomchristie
Copy link
Member Author

tomchristie commented Jan 6, 2020

Dropping this now in favour of #724, since that's our next step here.

I think that's essentially our 1.0 prelease, differences from there to 1.0 might include:

  • Switching the sync dispatch implementation from urlllib3 to an unasync'ed case of our built-in dispatch (Possibly even switched out to a third-party library as httpcore)
  • Refining the exception heirarchy, and ensuring we're completely happy with all the naming there that we're exposing as public API.

Headline stuff that's not API breaking, that we may want to keep on our horizons...

  • Streaming multipart implementation.
  • Socks proxy support.
  • Retry support.
  • Add chunk_size on streaming APIs.
  • Public Auth API for handling auth flow type cases. Public Auth API #732
  • Public dispatch API.
  • Perhaps mounting dispatchers to particular hosts?
  • Perhaps event hooks for on_request/on_response?
  • Perhaps a built-in concurrent requests API, esp. wrt. making async concurrent requests from a sync codebase.
  • Perhaps Twisted / Curio support?

@tomchristie tomchristie unpinned this issue Jan 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants