Skip to content
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 feature to bypass responses per demand #39

Closed
wants to merge 1 commit into from
Closed

Add feature to bypass responses per demand #39

wants to merge 1 commit into from

Conversation

ticosax
Copy link
Contributor

@ticosax ticosax commented Oct 15, 2014

This one is probably controversial.
bypass allows to bypass responses for selected requests.
Since requests is used in many libraries including elasticsearch clients for instance.
I would like in my test to NOT capture those calls in the context of @responses.activate that I'm using to capture calls to the web in the mean time.

Yep, I'm testing spaghetti code.

@dcramer
Copy link
Member

dcramer commented Oct 16, 2014

I think this makes sense as a feature. There are some complexities in that you can pass the other parameters to bypass (i.e. body) and it wont yell at you. I think we should fix that behavior so that the bypass method is extremely restrictive on what parameters it allows. That'd be as simple as changing the bypass signature to not be a partial.

Also do you think the name 'bypass' is clear enough? I wonder if something like 'allow' would more obviously state the intent.

@ticosax
Copy link
Contributor Author

ticosax commented Oct 16, 2014

You are right, I'll follow your recommendations.
Thx.

@ticosax
Copy link
Contributor Author

ticosax commented Oct 16, 2014

I rebased and added a tox.ini to test locally with all supported python versions

@aop
Copy link

aop commented Dec 10, 2014

Any idea when this would be merged? Ran into an issue with my code where I'd like to pass a request in the wild instead of catching it.

@ticosax
Copy link
Contributor Author

ticosax commented Apr 8, 2015

Has been rebased. this PR is now mergeable again.

@ticosax
Copy link
Contributor Author

ticosax commented May 22, 2015

Has been rebased. this PR is now mergeable again.

I will not abandon it so quickly.

@anentropic
Copy link
Contributor

+1 :)

@ticosax ticosax closed this Dec 9, 2015
@k4nar k4nar mentioned this pull request Dec 17, 2015
@bersace
Copy link

bersace commented Dec 21, 2015

@ticosax why did you closed this PR ? I do like the request.allow API which allow fine control on what is bypassed.

@bersace
Copy link

bersace commented Dec 21, 2015

@dcramer any clue on what is missing to merge this feature ?

@ticosax
Copy link
Contributor Author

ticosax commented Dec 22, 2015

I open this pull request in october 2014 and rebased it several time.
I'm fed up waiting,
Then I discover https://requests-mock.readthedocs.org/en/latest/ which I'm happily using.

@dcramer
Copy link
Member

dcramer commented Dec 22, 2015

To be honest, responses works fine. I built it to solve a problem and it solves it. I'm happy to let other people help maintain it, but it doesnt need to solve every problem in existence. If requests-mock works better for you, please use it.

@bersace
Copy link

bersace commented Dec 22, 2015

Thanks for the answers.

cc @toopy @k4nar

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.

None yet

5 participants