Same interface for for all http methods in api.py

[jtmiclat]

When using elasticsearch with httpx, I quickly realized that httpx.get does not take in json as a function argument. A quick look at api.py showed that not all methods have the same interface. Specifically, get/options/`head have fewer arguments than the others.

There are some arguments about that these methods should not send data. But I think as an HTTP client, users should not be restricted this way. I suggest that all methods in api.py should have the same interface. This also would solve compatibility with requests as it accepts json parameter in all its methods.

[sethmlarson]

Thanks for opening this issue, definitely an oversight!

We should fix the discrepancies and write a unit test using inspect.Signature to ensure this issue doesn't come up again.

Do you think you could tackle this issue?

[StephenBrown2]

I would counter that this is not particularly an oversight, but a deliberate choice based on the capabilities of HTTP, and also matching the requests api

Though the underlying request function does allow for arbitrary data/json/etc parameters, the others are more restricted based on the common expected use cases.

I would recommend referring to httpx.request for similar uses, that go outside the standard expectations for HTTP methods.

[thebigmunch]

and also matching the requests api

Actually, as the OP pointed out, httpx's behavior does not match the requests API. The requests API uses :param **kwargs: Optional arguments that request takes. for all the top level API functions, so, in turn, they all accept data/json/etc.

That being said, I actually agree with you that those functions should, in fact, not have params for things they don't actually support. And I agree with your suggestion of using httpx.request for those situations.

[sethmlarson]

@StephenBrown2 @thebigmunch You've both got good points, keeping our API trimmed in areas where parameters typically aren't found or have no meaning is a good way to steer our users to doing the right thing.

It looks like the .delete() function has the data, files, and json parameters, are those appropriate for the DELETE method?

I'd also like to mention that I'm still interested in a unit test that can track the API conformance of each method, especially for comparing across httpx, httpx.Client and httpx.AsyncClient so that we don't miss adding a parameter anywhere. :)

[jtmiclat]

Thanks for your comments!

I understand that having a more restrictive interface enforces better practices but I think that should be done in there server-side rather than enforcing it in client-side. As it excludes some using the convenient function for some servers, such as Elasticsearch. But if we do enforce the strict arguments we should add it to th compatibility docs https://github.com/encode/httpx/blob/master/docs/compatibility.md

Relevant excerpts I found:
RFC about the message body.
https://tools.ietf.org/html/rfc7230#page-28

Request message framing is independent of method semantics, even if the method does not define any use for a message body.

RFC about GET and request message.
https://tools.ietf.org/html/rfc7231#section-4.3.1

A payload within a GET request message has no defined semantics; sending a payload body on a GET request might cause some existing implementations to reject the request.

Would love to hear your feedback and final decision! @StephenBrown2 @thebigmunch @sethmlarson

[thebigmunch]

@jtmiclat

• The HTTP/1 specs were vague for some time leaving plenty of implementations that support this or don't.
• The HTTP/2 spec at least implies that a body shouldn't be sent.
• The OpenAPI 3 spec forbids GET, DELETE, and HEAD from having a request body.

I think that's reason enough not to directly support that in the specific methods.

[tomchristie]

I quickly realized that httpx.get does not take in json as a function argument. A quick look at api.py showed that not all methods have the same interface. Specifically, get/options/`head have fewer arguments than the others.

Indeed - this is deliberate. GET, HEAD, OPTIONS should not include a request body, and this won't be a supported usage with httpx.

Our lower-level APIs (eg. build up a request instance explicitly, and then send it) might conceivably be a good point at which to allow users to do broken things, but our top level API doesn't need to.

[sethmlarson]

For your case @jtmiclat you can use the Client.request() interface and set the method to "GET". If we're intending to not have body parameters available on methods that don't have bodies typically we should remove from .delete()

[tomchristie]

If we're intending to not have body parameters available on methods that don't have bodies typically we should remove from .delete()

GET is a "SHOULD NOT" case.
DELETE is a typically doesn't but may case.

I'd potentially be okay with us removing it from DELETE, but it's less clear there, and we're also following requests precedents here, I think.

[sethmlarson]

Relevant excerpt from RFC 7231 Section 4.3.5:

A payload within a DELETE request message has no defined semantics;
sending a payload body on a DELETE request might cause some existing
implementations to reject the request.

which as far as I can tell is the same as GET:

A payload within a GET request message has no defined semantics;
sending a payload body on a GET request might cause some existing
implementations to reject the request.

[thebigmunch]

Also, as I said here, OpenAPI forbids a body for GET, DELETE, and HEAD.

[tomchristie]

Ah, double checked HTTPbis, which supersedes the original RFC in a few places. And yes, we should drop em on .delete too. (Divergence from requests, but I’m totally okay with it)

https://tools.ietf.org/html/draft-ietf-httpbis-semantics-05#section-7.3.5