responses.add_callback triggers DeprecationWarning beyond user's control

[mblayman]

Steps to Reproduce

My company uses responses in combination with pytest with deprecation warnings treated as failures. The dependabot upgrade from 0.16.0 to 0.17.0 failed.

This test (which I've scrubbed to avoid sharing irrelevant and company sensitive details) triggers a DeprecationWarning upon execution.

    @responses.activate
    def test_something(self, caplog):
        # setup elided
    
        def request_callback(request):
            """Used to validate that the request correctly sets the status to deleted"""
            payload = json.loads(request.body)
            assert payload['StatusType'] == SOME_CONSTANT_TO_CHECK
            return 200, {}, json.dumps(SOME_RESPONSE_DATA)
    
       responses.add_callback(
            responses.POST, '/a/url/path', callback=request_callback
       )

Here is the warning:

E           DeprecationWarning: Argument 'match_querystring' is deprecated.
Use 'responses.matchers.query_param_matcher' or 'responses.matchers.query_string_matcher'

Expected Result

This test makes no use of match_querystring. I would not expect to see a deprecation warning.

Actual Result

A deprecation warning triggered.

I believe the reason is because add_callback sets match_querystring to False by default (see https://github.com/getsentry/responses/blob/0.17.0/responses/__init__.py#L719).

_should_match_querystring, the method where the deprecation warning is triggered, expects the more exact check of if match_querystring_argument is not None: (see https://github.com/getsentry/responses/blob/0.17.0/responses/__init__.py#L333)

My guess is that _should_match_querystring should be able to check the False value too to prevent disruption to the add_callback API.

Thanks for the help. Please let me know if I can do more to help or if more information is needed.

[beliaev-maksim]

@markstory
actually, this looks as architectural mistake that add_callback was setting False opposite to Response object
Changing default to None can cause issues when users have URL path with query but do not want to match query string.
Suppressing deprecation also looks strange. Solution for the user is to update tests and set manually match_querystring=None

or we can go a hard way and in next version release 1.0.0 with drop of multiple python versions and removing all deprecated parts. Then we can set default to ignore all query strings until it is explicitly defined via matcher

[beliaev-maksim]

@mblayman. @jankrepl

can you please check if https://github.com/beliaev-maksim/responses/tree/fix_464 fixes the issue for you

[bblommers]

@beliaev-maksim I'm still seeing the warning on this branch, using pip install https://github.com/beliaev-maksim/responses/archive/refs/heads/fix_464.zip

[beliaev-maksim]

@bblommers
And you don't have explicit definition of the argument in add_callback ?

Can you please provide minimal reproducible?

[bblommers]

@bblommers And you don't have explicit definition of the argument in add_callback ?

Ah, apologies - we do explicitly add that argument, so the warning is to be expected.

I cannot reproduce the warning after removing the argument!

[beliaev-maksim]

@bblommers
correct,
if you need to match query strings, please switch to matchers.
This provides explicit comparison of requests

[jankrepl]

@mblayman. @jankrepl

can you please check if https://github.com/beliaev-maksim/responses/tree/fix_464 fixes the issue for you

Seems to be working:) Thanks for the quick help:)

[beliaev-maksim]

@jankrepl
thanks for checking!

[mblayman]

Thank you for fixing this quickly!

[beliaev-maksim]

@mblayman
always glad to help :)