httpx.Request don't take the same arguments #794

Activity

1. L0ric0

   changed the title [-]httpx.Request dosn't have the same [/-] [+]httpx.Request don't take the same arguments[/+] on Jan 28, 2020
2. florimondmanca commented on Jan 28, 2020

   

   florimondmanca

   on Jan 28, 2020

   Contributor

   More actions

   Happy to review any PRs that document which parameters aren’t present on our Request class compared to Requests. :)

   From what I can see these are:

   • files (I believe this is handled via data, isn’t it?)
   • hooks
3. florimondmanca

   added

   docsChanges to the documentationChanges to the documentation

   good first issueGood for newcomersGood for newcomers

   on Jan 28, 2020
4. StephenBrown2 commented on Jan 28, 2020

   

   StephenBrown2

   on Jan 28, 2020

   Contributor

   More actions

   files is there, but auth isn't.
   

   httpx/httpx/_models.py

   Lines 591 to 604 in 82dc6f3

   	class Request:
   	def __init__(
   	self,
   	method: str,
   	url: typing.Union[str, URL],
   	*,
   	params: QueryParamTypes = None,
   	headers: HeaderTypes = None,
   	cookies: CookieTypes = None,
   	data: RequestData = None,
   	files: RequestFiles = None,
   	json: typing.Any = None,
   	stream: ContentStream = None,
   	):

   However, auth is available on the request method:

   httpx/httpx/_api.py

   Lines 18 to 34 in 82dc6f3

   	def request(
   	method: str,
   	url: URLTypes,
   	*,
   	params: QueryParamTypes = None,
   	data: RequestData = None,
   	files: RequestFiles = None,
   	json: typing.Any = None,
   	headers: HeaderTypes = None,
   	cookies: CookieTypes = None,
   	auth: AuthTypes = None,
   	timeout: TimeoutTypes = DEFAULT_TIMEOUT_CONFIG,
   	allow_redirects: bool = True,
   	verify: VerifyTypes = True,
   	cert: CertTypes = None,
   	trust_env: bool = True,
   	) -> Response:

   Hooks are discussed here: #790
5. florimondmanca commented on Jan 28, 2020

   

   florimondmanca

   on Jan 28, 2020

   Last edited by florimondmanca

   Contributor

   More actions

   but auth isn't

   Not immediately obvious to me that this was a deliberate design choice, or just the way things ended up being implemented. (We're passing auth in Client.send(), but we probably could very well move it onto a request.auth attribute.) As a matter of fact, we do support per-request auth, so… Maybe there's room for retro-fitting to the Requests API here?
6. florimondmanca

   added

   requests-compatIssues related to Requests backwards compatibilityIssues related to Requests backwards compatibility

   on Jan 28, 2020
7. lovelydinosaur commented on Jan 29, 2020

   

   lovelydinosaur

   on Jan 29, 2020

   Member

   More actions

   Not immediately obvious to me that this was a deliberate design choice, or just the way things ended up being implemented. (We're passing auth in Client.send(), but we probably could very well move it onto a request.auth attribute.) As a matter of fact, we do support per-request auth, so… Maybe there's room for retro-fitting to the Requests API here?

   I think we proabably want to keep things as they are. When I started out the "Requests Compatability" the intention was to document anything that differed across the Basic and Advanced usage guides, rather than that lower level bits of API would match all the way through.

   It makes sense to me that our auth parameter would be on the send() method, since it's not something that we can always just call into to build some extra headers on the request method, it actually alters the sending flow.

   I think we might be okay with a slightly general "Not all stuff in the lower level bits of API might match up exactly", giving a couple of examples? Or else perhaps a section for low level API differences, where we briefly list any notable cases. (Eg. I also haven't reviewed how our build_request/send compares to the requests equivelent.)

   👍React with 👍1Reacted by Vishes
8. StephenBrown2 commented on Jan 29, 2020

   

   StephenBrown2

   on Jan 29, 2020

   Contributor

   More actions

   Agreed, I think it was only top-level compatibility (httpx.HTTP_VERB) that was shot for, and other things have come along naturally as they made sense to include.
9. Pradhvan commented on Jun 14, 2020

   

   Pradhvan

   on Jun 14, 2020

   More actions

   @florimondmanca hey, you should close this PR as #823 is already been merged. 😄
10. florimondmanca commented on Jun 14, 2020

    

    florimondmanca

    on Jun 14, 2020

    Contributor

    More actions

    Thanks for the heads up @Pradhvan!
11. florimondmanca

    closed this as completedon Jun 14, 2020