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

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

Closed
L0ric0 opened this issue Jan 28, 2020 · 7 comments · Fixed by #823
Closed

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

L0ric0 opened this issue Jan 28, 2020 · 7 comments · Fixed by #823
Labels
docs Changes to the documentation good first issue Good for newcomers requests-compat Issues related to Requests backwards compatibility

Comments

@L0ric0
Copy link

L0ric0 commented Jan 28, 2020

the httpx.Request takes (method, url, [params], [data], [json], [headers], [cookies])
and the requests.Request takes (method=None, url=None, headers=None, files=None, data={}, params={}, auth=None, cookies=None, hooks=None)

this should be pointed out in the documentation or implemented

@L0ric0 L0ric0 changed the title httpx.Request dosn't have the same httpx.Request don't take the same arguments Jan 28, 2020
@florimondmanca
Copy link
Member

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

@florimondmanca florimondmanca added docs Changes to the documentation good first issue Good for newcomers labels Jan 28, 2020
@StephenBrown2
Copy link
Contributor

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

@florimondmanca
Copy link
Member

florimondmanca commented Jan 28, 2020

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?

@florimondmanca florimondmanca added the requests-compat Issues related to Requests backwards compatibility label Jan 28, 2020
@tomchristie
Copy link
Member

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.)

@StephenBrown2
Copy link
Contributor

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.

@Pradhvan
Copy link

@florimondmanca hey, you should close this PR as #823 is already been merged. 😄

@florimondmanca
Copy link
Member

Thanks for the heads up @Pradhvan!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Changes to the documentation good first issue Good for newcomers requests-compat Issues related to Requests backwards compatibility
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants