Skip to content

Add proxies parameter to top-level API functions - #1198

Merged
j178 merged 6 commits into
masterfrom
top-level-proxies
Aug 20, 2020
Merged

Add proxies parameter to top-level API functions#1198
j178 merged 6 commits into
masterfrom
top-level-proxies

Conversation

@j178

@j178j178 commented Aug 19, 2020

Copy link
Copy Markdown
Contributor

Closes #1195

Should I add tests to cover these parameters in top-level API functions?

florimondmanca
florimondmanca reviewed Aug 19, 2020
View reviewed changes

@florimondmancaflorimondmanca left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks!

tests

Sure! Perhaps a single test against httpx.request is enough here, but up to you!

Comment threadhttpx/_api.py Outdated
Comment threaddocs/advanced.md Outdated
@j178

j178 commented Aug 20, 2020

Copy link
Copy Markdown
ContributorAuthor

I find it a little difficult to test whether these parameters passed to top-level API functions are working as expected, do you have any ideas?

@florimondmanca

florimondmanca commented Aug 20, 2020

Copy link
Copy Markdown
Contributor

I see! Indeed I went through our proxy tests here and they're mostly about the proxy routing functionality - no existing test actually integrates with HTTPCore and a live proxy. Well no sweat then, I think it's alright (we certainly don't have tests for all options in the top level API anyway).

florimondmanca
florimondmanca approved these changes Aug 20, 2020
View reviewed changes

@florimondmancaflorimondmanca left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

Found a tiny typo

Comment threaddocs/compatibility.md Outdated
@j178
j178 merged commit 84ca201 into masterAug 20, 2020
@j178
j178 deleted the top-level-proxies branch August 20, 2020 07:55
@lovelydinosaurlovelydinosaur mentioned this pull request Aug 21, 2020
Merged
@rking32rking32 mentioned this pull request Nov 7, 2021
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@florimondmancaflorimondmancaflorimondmanca approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Allow proxies in httpx.request() high-level API

2 participants

@j178@florimondmanca