-
-
Notifications
You must be signed in to change notification settings - Fork 354
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
Add support for drop-in replacement Niquests when installed #725
Conversation
responses remain attached to Requests indefinitely, but there is an opportunity to support another http client fully compatible with Requests, namely Niquests. The commit add a thin layer of compat logic to import Niquests instead of Requests if present in the environment. At the time of this commit, Niquests is not likely to be present in one's environment unless that is what the user wanted. An escape hatch is also present, in case both Niquests and Requests are present and the user wants to mock Requests only. Setting "RESPONSES_USE_REQUESTS" with any value inside revert to using Requests only. Unfortunately, for the time being, no simple ways emerge to support both library at once.
@@ -469,7 +471,9 @@ def _url_matches(self, url: "_URLPatternType", other: str) -> bool: | |||
if _has_unicode(url): | |||
url = _clean_unicode(url) | |||
|
|||
return _get_url_and_path(url) == _get_url_and_path(other) | |||
return _get_url_and_path(url) == _get_url_and_path( | |||
_ensure_url_default_path(other) # type: ignore[arg-type] |
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.
To explain this change:
Requests, in "prepare_url" does things that end to "enforce" a trailing "/". Which isn't enforced by Niquests.
So, we have to enforce consistency across both library. here, only Niquests is affected. Not Requests.
@@ -822,8 +844,11 @@ def send( | |||
verify=True, | |||
cert=None, | |||
proxies=None, | |||
**kwargs, |
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.
Niquests adapters have a few more arguments. So this is mainly for the compat purposes. By my reading, it does not affect your initial test.
@@ -1058,7 +1094,7 @@ def run(): | |||
body="ok", | |||
stream=responses_stream, | |||
) | |||
session = requests.session() | |||
session = requests.Session() |
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.
the session
with s
lowercased is actually deprecated since a long time in Requests (since v1 if I recall correctly).
and removed in Niquests.
@@ -5,6 +5,7 @@ envlist = py38,py39,py310,py311,py312,mypy,precom | |||
filterwarnings = | |||
error | |||
default::DeprecationWarning | |||
ignore:Passing msg:DeprecationWarning |
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.
this deprecation happen in Niquests low-level replacement of urllib3. Nothing to be concerned about.
|
||
from responses.matchers import json_params_matcher as _json_params_matcher | ||
from responses.matchers import query_string_matcher as _query_string_matcher | ||
from responses.matchers import urlencoded_params_matcher as _urlencoded_params_matcher | ||
from responses.registries import FirstMatchRegistry | ||
|
||
from ._compat import MOCKED_LIBRARY |
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.
After thinking about it, I did not find a perfect way to support Niquests "perfectly" without doing the compat import switch.
I did found that this way offer more stability.
Close #710
Close jawah/niquests#109
responses remain attached to Requests indefinitely, but there is an opportunity to support another http client fully compatible with Requests, namely Niquests.
The commit add a thin layer of compat logic to import Niquests instead of Requests if present in the environment. At the time of this commit, Niquests is not likely to be present in one's environment unless that is what the user wanted.
An escape hatch is also present, in case both Niquests and Requests are present and the user wants to mock Requests only. Setting "RESPONSES_USE_REQUESTS" with any value inside revert to using Requests only.
Unfortunately, for the time being, no simple ways emerge to support both library at once.
Comments will be attached to justify every changes. I tried to make it as light as possible.
Of course, if anything started to make your life harder, I will surely come and help as much as I can.
This would offer Niquests users, a reliable way to mock http responses.
Lastly, Niquests does have async via
AsyncSession
, that is a mirror of its synchronous counterpart, and for now, I have no idea how it would be conceivable to support it here. It's not the topic of this PR. Just to brought it up.regards,