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

API Recommendation: using HTTPRequest with HTTPClient is confusing #16

Open
jentfoo opened this issue Feb 16, 2018 · 1 comment
Open

Comments

@jentfoo
Copy link
Member

jentfoo commented Feb 16, 2018

I think this comes down to several things:

  1. I think we should remove HTTPAddress. I understand why this class makes sense internally for connection management. However it complicates the API. I think most people expect to just use the URL object (which the API technically also accepts). Understanding the differences between them at an API level is confusing, even more so because of:

  2. The HTTPRequestBuilder is confusing. It can accept a String path, a HTTPAddress as well as a URL. It is not obvious what is being set and needs to be set in all cases, particularly because when making a request with on in the HTTPClient a HTTPAddress must also be specified. But wait, was it not set earlier? Does it need to match? What if we are using a URL?

I think the HTTP client should never accept the host, instead getting it either from a URL or the request. Allowing there never to be any confusion.

@lwahlmeier
Copy link
Member

We do accept a URL, granted you cant change the type of request (its always a get).

The reason for the HTTPAddress is because an HTTPRequest does not actually contain the information needed to know where to connect. The builder can take in that info but its not put in the request, its only used for buildHTTPAddress. Granted the host header could be added/used but thats not required for a request so it starts getting complicated.

This starts to get more complicated when you want to do things like send an HTTPRequest to a different host then the one in the host headers, and I dont think requiring HTTPRequest to have a ip/host even outside of the headers is a good solution either. I guess we could maybe have something that wraps the 2 like an HTTPServerRequest or something.

I am all for coming up with another way of doing it, and maybe moving back to a more standard:
(URL, RequestType, ExtraHeaders, body) is better, but its not exact enough for my needs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants