add httpx support #523

herdigiorgi · 2020-04-24T23:50:58Z

This PR adds support for httpx.

The stub and tests are heavily based in the aiohttp implementation. You won't see any synchronous implementation because it was not necessary.

Not sure if I edited the tox file correctly. (:

fullonic · 2020-04-27T14:25:08Z

hey @herdigiorgi ! Thanks for the PR. 👍
I tried your PR branch in order to see if would solve a issue I got when turning sync function with async, however I couldn't solve it.

Maybe I'm doing something wrong, so bellow is a dummy example of what I'm trying to test and save into a cassette. I have included a sync and async version, because the sync version works.

import pytest
import httpx

def urls():
    base_url = "https://github.com/kevin1024/vcrpy/issues"
    return (f"{base_url}/{i}" for i in range(1, 5))

async def async_fetch_url():
    session = httpx.AsyncClient()
    for url in urls():
        page = await session.get(url)
        # do stuff with page

@pytest.mark.vcr()
@pytest.mark.asyncio
async def test_async_url_fetch():
    await async_fetch_url()

def sync_fetch_url():
    for url in urls():
        page = httpx.get(url)
        # do stuff with page

@pytest.mark.vcr()
def test_sync_url_fetch():
    sync_fetch_url()

After installing your branch version, I was able to save the cassette, but when running again the same test I got the follow error:

    (...)
    E       vcr.errors.UnhandledHTTPRequestError: "
The cassette ('./cassettes/test_async_url_fetch.yaml') doesn't contain the request 
(<Request (GET) https://github.com/kevin1024/vcrpy/issues/1>) asked for"

    venv/lib/python3.8/site-packages/vcr/cassette.py:265: UnhandledHTTPRequestError

The libraries version:

pytest==5.4.1
pytest-asyncio==0.11.0
pytest-vcr==1.0.2
httpx==0.12.1

Also I try to ran the above code example using asyncio, without pytest in the middle, but without success. Same error.
As you mention that your PR is based on the stub implementation of aiohttp, I gave a try with the aiohttp, however I got the same issue.

Any suggestion or workaround? Thanks

XaviTorello

Thanks for this PR, it's really interesting, but some scenarios should be reviewed, like the issue exposed by @fullonic.

Many thanks @herdigiorgi 🍻

XaviTorello · 2020-04-27T15:33:22Z

vcr/stubs/httpx_stubs.py

+
+    while 300 <= response.status_code <= 399:
+        location = response.headers["location"]
+        next_url = URL(str(response.url)).with_path(location)


In the example provided by @fullonic this breaks the composition of the next_url.

Result of the issued scenario described by @fullonic:

response.url: URL('https://github.com/kevin1024/vcrpy/issues/1')

response.headers.get('location'): https://github.com/kevin1024/vcrpy/pull/1

location: URL('https://github.com/https://github.com/kevin1024/vcrpy/pull/1')

It's really needed? Why we couldn't just use the location reached from the response headers and validate if it's an absolute or relative?

Maybe something like this can help:

next_url = URL(response.headers["location"]) if not next_url.is_absolute(): # could be simplified, it's just a quick example :) next_url = URL(str(response.url)).with_path(next_url)

Thanks! (: I fixed this, and also I stop the loop if there is no location as suggested in #511.

herdigiorgi · 2020-04-28T00:47:04Z

Today while using this I noticed other problems 😅, so that's why you will find more commits.

herdigiorgi · 2020-04-28T01:04:33Z

mmm... I will fix CI tomorrow.

fullonic · 2020-04-28T07:10:11Z

Thanks @XaviTorello and @herdigiorgi . I ran again the sample code with our new changes and it works! Maybe we could apply the same changes to the aiohttp stubs to fixed as well?

codecov-io · 2020-04-28T22:40:24Z

Codecov Report

Merging #523 into master will decrease coverage by 4.54%.
The diff coverage is 44.96%.

@@            Coverage Diff             @@
##           master     #523      +/-   ##
==========================================
- Coverage   89.41%   84.86%   -4.55%     
==========================================
  Files          24       25       +1     
  Lines        1549     1619      +70     
  Branches      216      221       +5     
==========================================
- Hits         1385     1374      -11     
- Misses        135      215      +80     
- Partials       29       30       +1

Impacted Files	Coverage Δ
vcr/stubs/boto3_stubs.py	`56.25% <0.00%> (+0.69%)`	⬆️
vcr/stubs/httpx_stubs.py	`0.00% <0.00%> (ø)`
vcr/stubs/tornado_stubs.py	`0.00% <0.00%> (ø)`
vcr/filters.py	`93.61% <66.66%> (ø)`
vcr/patch.py	`83.39% <68.75%> (-0.90%)`	⬇️
vcr/cassette.py	`96.17% <100.00%> (-1.00%)`	⬇️
vcr/config.py	`94.96% <100.00%> (-0.10%)`	⬇️
vcr/errors.py	`100.00% <100.00%> (ø)`
vcr/matchers.py	`96.05% <100.00%> (+0.05%)`	⬆️
vcr/persisters/filesystem.py	`100.00% <100.00%> (ø)`
... and 11 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2e5fdd3...2b5a2e5. Read the comment docs.

herdigiorgi · 2020-04-28T22:44:35Z

Now CI passes 🎉

herdigiorgi · 2020-04-28T22:47:26Z

Maybe we could apply the same changes to the aiohttp stubs to fixed as well?

There is already a PR that only needs to be merged: https://github.com/kevin1024/vcrpy/pull/511/files. I think it wasn't merged because there isn't an active aiohttp maintainer.

2m · 2020-05-05T15:13:51Z

I started using this branch in one project and noticed that cookies were not extracted to the client session cookie jar anymore when requests/responses were replayed from cassettes.

Added tests for that situation and fixed it in PRs to this PR branch:
herdigiorgi#1
herdigiorgi#2

Also enabled running Travis CI for httpx. That also revealed that httpx is not available for Python 3.5. Can we disable that when testing vcrpy with httpx?

jairhenrique · 2020-06-29T19:48:12Z

@herdigiorgi @XaviTorello what needs to be done on this pull request to be merged?

herdigiorgi · 2020-06-29T20:52:32Z

herdigiorgi @XaviTorello what needs to be done on this pull request to be merged?

@jairhenrique I'm currently using this branch 😄, so from from my side I think it is done.

jairhenrique · 2020-06-29T20:56:17Z

@kevin1024 can you help here?

jairhenrique · 2020-06-30T11:53:31Z

@IvanMalison can you help this to go to master?

jairhenrique · 2020-06-30T12:02:19Z

@herdigiorgi can you add httpx to Compatibility list on docs?! https://github.com/kevin1024/vcrpy/blob/4a8e80ee3e288c61a85a90d7f13c83d9e9a4db2e/docs/installation.rst

herdigiorgi · 2020-07-01T00:24:55Z

@jairhenrique httpx was added to the list: ab55d88 .

kevin1024 · 2020-07-02T23:05:01Z

This looks awesome. Any way to get Travis to pass before I merge?

kevin1024 · 2020-07-15T18:52:54Z

Could I get a rebase on this? If tests pass I'll merge.

This is required because previous extraction code is now patched out by vcpy. Also handle headers with same key in responses.

Because the recoded requests are using original value. Changing the value on redirect prevents finding the matching response.

jairhenrique · 2020-07-15T19:48:38Z

@herdigiorgi httpx doesn't support python 3.5.
It would be nice to replace httpbin to mockbin

herdigiorgi · 2020-07-16T14:03:58Z

fyi, I will take care of the errors in the next 2 days.

Ignore utf8 decoding errors in content

- Use mockbin for redirections. - Mock synchronous requests too.

codecov-commenter · 2020-07-18T23:51:54Z

Codecov Report

Merging #523 into master will decrease coverage by 4.98%.
The diff coverage is 41.75%.

@@            Coverage Diff             @@
##           master     #523      +/-   ##
==========================================
- Coverage   89.01%   84.03%   -4.99%     
==========================================
  Files          24       26       +2     
  Lines        1539     1704     +165     
  Branches      212      239      +27     
==========================================
+ Hits         1370     1432      +62     
- Misses        139      237      +98     
- Partials       30       35       +5

Impacted Files	Coverage Δ
vcr/stubs/httpx_stubs.py	`0.00% <0.00%> (ø)`
vcr/patch.py	`82.74% <46.15%> (-1.62%)`	⬇️
vcr/stubs/aiohttp_stubs.py	`94.30% <88.05%> (ø)`
vcr/cassette.py	`96.20% <100.00%> (+0.03%)`	⬆️
vcr/config.py	`95.00% <100.00%> (+0.03%)`	⬆️
vcr/record_mode.py	`100.00% <100.00%> (ø)`
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a249781...16dc385. Read the comment docs.

herdigiorgi · 2020-07-18T23:57:33Z

Tests pass now. (:

jairhenrique · 2020-07-24T19:32:48Z

🚀

@kevin1024 can you bump a new version with this?

kevin1024 · 2020-07-31T14:54:17Z

OK, it's released! Thanks so much for your contributions.

XaviTorello suggested changes Apr 27, 2020

View reviewed changes

herdigiorgi force-pushed the httpx branch from 889faf9 to 2b5a2e5 Compare April 28, 2020 22:27

ke-zhang-rd mentioned this pull request May 12, 2020

Need to consider a new test approach OR work with vcrpy NSLS-II/olog#11

Closed

jairhenrique approved these changes Jul 1, 2020

View reviewed changes

jairhenrique mentioned this pull request Jul 15, 2020

Use mockbin to fix redirect test #545

Merged

herdigiorgi and others added 9 commits July 15, 2020 16:01

add httpx support

7a1b3be

add missing pytest-asyncio dep for httpx

b96a134

make it work with gzipped data

49005d6

fix redirection url

f68c8f9

test that works behind proxy

9ce191b

fix linting errors in test_httpx.py

0f589ee

Run tests on Travis CI

b823521

Extract cookies to client on every response

59e8840

This is required because previous extraction code is now patched out by vcpy. Also handle headers with same key in responses.

Reuse value from Location header on redirect

89a7b29

Because the recoded requests are using original value. Changing the value on redirect prevents finding the matching response.

2m and others added 2 commits July 15, 2020 16:01

Fix linter

b4500df

list httpx in the compatibility list

f80ee12

herdigiorgi force-pushed the httpx branch from ab55d88 to f80ee12 Compare July 15, 2020 19:01

Ignore utf8 decoding errors in content

0a3ebbb

herdigiorgi added 3 commits July 16, 2020 11:07

Merge pull request #3 from newah/httpx

7c4c3b7

Ignore utf8 decoding errors in content

update tests

f3d31a3

- Use mockbin for redirections. - Mock synchronous requests too.

support allow_redirects option

16dc385

herdigiorgi force-pushed the httpx branch from 8f8167f to 16dc385 Compare July 18, 2020 23:31

kevin1024 merged commit f387950 into kevin1024:master Jul 24, 2020

tyler-8 mentioned this pull request Jan 5, 2022

Add httpx to compatible HTTP client list #626

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add httpx support #523

add httpx support #523

herdigiorgi commented Apr 24, 2020

fullonic commented Apr 27, 2020

XaviTorello left a comment

XaviTorello Apr 27, 2020 •

edited

herdigiorgi Apr 28, 2020

herdigiorgi commented Apr 28, 2020

herdigiorgi commented Apr 28, 2020

fullonic commented Apr 28, 2020

codecov-io commented Apr 28, 2020 •

edited

herdigiorgi commented Apr 28, 2020

herdigiorgi commented Apr 28, 2020

2m commented May 5, 2020

jairhenrique commented Jun 29, 2020

herdigiorgi commented Jun 29, 2020 •

edited

jairhenrique commented Jun 29, 2020

jairhenrique commented Jun 30, 2020

jairhenrique commented Jun 30, 2020

herdigiorgi commented Jul 1, 2020

kevin1024 commented Jul 2, 2020

kevin1024 commented Jul 15, 2020

jairhenrique commented Jul 15, 2020

herdigiorgi commented Jul 16, 2020

codecov-commenter commented Jul 18, 2020 •

edited

herdigiorgi commented Jul 18, 2020

jairhenrique commented Jul 24, 2020

kevin1024 commented Jul 31, 2020

add httpx support #523

add httpx support #523

Conversation

herdigiorgi commented Apr 24, 2020

fullonic commented Apr 27, 2020

XaviTorello left a comment

Choose a reason for hiding this comment

XaviTorello Apr 27, 2020 • edited

Choose a reason for hiding this comment

herdigiorgi Apr 28, 2020

Choose a reason for hiding this comment

herdigiorgi commented Apr 28, 2020

herdigiorgi commented Apr 28, 2020

fullonic commented Apr 28, 2020

codecov-io commented Apr 28, 2020 • edited

Codecov Report

herdigiorgi commented Apr 28, 2020

herdigiorgi commented Apr 28, 2020

2m commented May 5, 2020

jairhenrique commented Jun 29, 2020

herdigiorgi commented Jun 29, 2020 • edited

jairhenrique commented Jun 29, 2020

jairhenrique commented Jun 30, 2020

jairhenrique commented Jun 30, 2020

herdigiorgi commented Jul 1, 2020

kevin1024 commented Jul 2, 2020

kevin1024 commented Jul 15, 2020

jairhenrique commented Jul 15, 2020

herdigiorgi commented Jul 16, 2020

codecov-commenter commented Jul 18, 2020 • edited

Codecov Report

herdigiorgi commented Jul 18, 2020

jairhenrique commented Jul 24, 2020

kevin1024 commented Jul 31, 2020

XaviTorello Apr 27, 2020 •

edited

codecov-io commented Apr 28, 2020 •

edited

herdigiorgi commented Jun 29, 2020 •

edited

codecov-commenter commented Jul 18, 2020 •

edited