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

Move httpx monkeypatching to a lower layer to allow recording request generated by auth flows. #742

Closed
wants to merge 2 commits into from

Conversation

ionelmc
Copy link

@ionelmc ionelmc commented Jul 10, 2023

Not sure if i wrote the test correctly but that test suite is way over my head.

I would like to note that the monkeypatching should be moved even lower but then there's some complicated redirect handling that I can't untangle.

Should fix #684.

@ionelmc
Copy link
Author

ionelmc commented Jul 10, 2023

I think someone needs to push some buttons to get CI approved.

@codecov-commenter
Copy link

Codecov Report

Merging #742 (3cb60c2) into master (c95b726) will decrease coverage by 0.16%.
The diff coverage is 100.00%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##           master     #742      +/-   ##
==========================================
- Coverage   90.63%   90.47%   -0.16%     
==========================================
  Files          28       28              
  Lines        1794     1796       +2     
  Branches      268      322      +54     
==========================================
- Hits         1626     1625       -1     
- Misses        133      135       +2     
- Partials       35       36       +1     
Impacted Files Coverage Δ
vcr/patch.py 88.42% <100.00%> (ø)
vcr/stubs/httpx_stubs.py 95.95% <100.00%> (-1.98%) ⬇️

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more


with vcr.use_cassette(yml):
with do_request() as client:
client("GET", url, auth=AuthWithExtraRequest())
Copy link
Collaborator

Choose a reason for hiding this comment

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

The default value for follow_redirects is False on httpx client.
I think if you call the client with client("GET", url, auth=AuthWithExtraRequest(), follow_redirects=True) we don't need to overwrite a private method.

Copy link
Author

Choose a reason for hiding this comment

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

I don't really understand what you wrote. If you think something else works please try it.

@parkerhancock
Copy link
Contributor

Also fixed in #784

@parkerhancock
Copy link
Contributor

I suggest we close this one in view of #784

@graingert
Copy link
Collaborator

Looks like #784 got merged

@graingert graingert closed this Dec 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

httpx auth flow is skipped by replay?
5 participants