-
Notifications
You must be signed in to change notification settings - Fork 389
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
Keep the same request method when following redirects for non 303 responses in httpx #552
base: master
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## master #552 +/- ##
==========================================
- Coverage 89.41% 83.98% -5.43%
==========================================
Files 24 26 +2
Lines 1549 1705 +156
Branches 216 239 +23
==========================================
+ Hits 1385 1432 +47
- Misses 135 238 +103
- Partials 29 35 +6
Continue to review full report at Codecov.
|
vcr/stubs/httpx_stubs.py
Outdated
@@ -108,7 +108,8 @@ def _play_responses(cassette, request, vcr_request, client, kwargs): | |||
if not next_url: | |||
break | |||
|
|||
vcr_request = VcrRequest("GET", next_url, None, dict(response.headers)) | |||
method = request.method if response.status_code != 303 else "GET" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its tricky to know what would be the best logic here because it is not strictly mandated by the HTTP spec.
However according to https://developer.mozilla.org/en-US/docs/Web/HTTP/Redirections the only status code that always changes to GET
method is 303
. So this LGTM!
@@ -108,7 +108,11 @@ def _play_responses(cassette, request, vcr_request, client, kwargs): | |||
if not next_url: | |||
break | |||
|
|||
vcr_request = VcrRequest("GET", next_url, None, dict(response.headers)) | |||
# Even though 302 shouldn't change request's method, browsers like Chromium |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why does what browsers do here matter?
df3997c
to
34d5384
Compare
I suggest we close this one in view of #784 |
Request method should be kept the same whenever redirect is not 302 and 303