Skip to content

Fix compatibility with httpx 0.28.0#278

Merged
lundberg merged 5 commits intolundberg:masterfrom
ndhansen:master
Dec 19, 2024
Merged

Fix compatibility with httpx 0.28.0#278
lundberg merged 5 commits intolundberg:masterfrom
ndhansen:master

Conversation

@ndhansen
Copy link
Copy Markdown
Contributor

@ndhansen ndhansen commented Nov 29, 2024

Prior to httpx version 0.28.0, request methods were implicitly converted from bytes to strings. In version 0.28.0 that is no longer the case.

Since httpx request objects expect strings for the method field, and httpcore request objects enforce bytes, I've copied the conversion from bytes to strings from the previous httpx version in to respx.

I didn't add tests because there are no explicit tests on the to_httpx_request method, but I can if you'd like.

Fixes #277.

respx/mocks.py Outdated
Comment on lines +301 to +302
request.method.decode("ascii").upper()
if isinstance(request.method, bytes)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Should a DeprecationWarning be raised for bytes so that codebases can be updated and this support can eventually be dropped?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Yes, I'd expect respx to follow the same argument conventions as httpx, and the project shouldn't try and maintain accepting bytes here if httpx has dropped support.

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.

Should the minimum HTTPX version also be bumped to match?

respx/setup.py

Line 44 in 366dd0b

install_requires=["httpx>=0.21.0"],

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested as ndhansen#1.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Should the minimum HTTPX version also be bumped to match?

I don’t think so, the library should still be compatible with older versions of httpx (they used to accept bytes for the method kwarg).

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The bytes come from httpcore, and we’re only getting bytes because we’re hooking the mock after httpcore issued the request. So there’s nothing users can do about this warning.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I agree @mjpieters .. respx has always aligned with httpx and dropped support for old api's.

So, yes @hugovk, I think we should bump the required version of httpx .. respx is still compatible with older versions of httpx by using older versions of respx 😉

@francoisfreitag
Copy link
Copy Markdown

Commit that changes httpx._model.Request constructor: encode/httpx@6622553

@jbw-vtl
Copy link
Copy Markdown

jbw-vtl commented Dec 9, 2024

Quite keen on this as well, anything possible to help with getting this over the line?

@bnavigator
Copy link
Copy Markdown

I needed to add this, but of course it is not backwards compatible:

--- respx-0.21.1.orig/tests/test_api.py
+++ respx-0.21.1/tests/test_api.py
@@ -214,7 +214,7 @@ async def test_content_variants(client,
             {"X-Foo": "bar"},
             {
                 "Content-Type": "application/json",
-                "Content-Length": "14",
+                "Content-Length": "13",
                 "X-Foo": "bar",
             },
         ),
@@ -223,7 +223,7 @@ async def test_content_variants(client,
             {"Content-Type": "application/json; charset=utf-8", "X-Foo": "bar"},
             {
                 "Content-Type": "application/json; charset=utf-8",
-                "Content-Length": "14",
+                "Content-Length": "13",
                 "X-Foo": "bar",
             },
         ),
@@ -322,19 +322,19 @@ async def test_callable_content(client):
         assert request.called is True
         assert async_response.status_code == 200
         assert async_response.text == "hello world."
-        assert request.calls[-1][0].content == b'{"x": "."}'
+        assert request.calls[-1][0].content == b'{"x":"."}'
 
         respx_mock.reset()
         sync_response = httpx.post("https://foo.bar/jonas/", json={"x": "!"})
         assert request.called is True
         assert sync_response.status_code == 200
         assert sync_response.text == "hello jonas!"
-        assert request.calls[-1][0].content == b'{"x": "!"}'
+        assert request.calls[-1][0].content == b'{"x":"!"}'
 
 
 async def test_request_callback(client):
     def callback(request, name):
-        if request.url.host == "foo.bar" and request.content == b'{"foo": "bar"}':
+        if request.url.host == "foo.bar" and request.content == b'{"foo":"bar"}':
             return respx.MockResponse(
                 202,
                 headers={"X-Foo": "bar"},

Copy link
Copy Markdown
Owner

@lundberg lundberg left a comment

Choose a reason for hiding this comment

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

Looking good! Just remove the import changes in patterns.py

@lundberg
Copy link
Copy Markdown
Owner

I needed to add this, but of course it is not backwards compatible:

I have no problem with this .. only tests

@lundberg
Copy link
Copy Markdown
Owner

lundberg commented Dec 19, 2024

@ndhansen I've merged #280 to simplify .. rebase this PR and fix my comment about linting

.. remove the httpx upper bound <0.28.0 in setup.py and we're ready to merge 👏

@ndhansen
Copy link
Copy Markdown
Contributor Author

Done! Thanks for taking a look!

The tests are failing on test_remote.py, but I noticed that test_remote.py when run on it's own it seems to succeed. I might need to take a look at why that's happening and what is leaking between tests.

@userlerueda
Copy link
Copy Markdown

We really need this fix. Can someone help? I want to avoid hardcoding my dependency to respx = { git = "https://github.com/ndhansen/respx.git" } but it works with @ndhansen version

@codecov
Copy link
Copy Markdown

codecov bot commented Dec 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (81d90c0) to head (4d19849).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##            master      #278   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           22        22           
  Lines         2914      2915    +1     
  Branches       192       192           
=========================================
+ Hits          2914      2915    +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@lundberg lundberg merged commit 179c651 into lundberg:master Dec 19, 2024
@lundberg
Copy link
Copy Markdown
Owner

Version 0.22.0 now released

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.

Incompatibility with httpx 0.28.0

9 participants