-
Notifications
You must be signed in to change notification settings - Fork 71
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
1.9.0/1.9.1 - Breaks compatibility by double quoting characters. #170
Comments
I'd prefer to take the option to update this to use the same function as requests (as per #169), rather than revert. @m-melis @ktdreyer @OrangeDog @Cheaterman - can you chime in if this will work for you? |
Sounds good to me - the way I discovered the original issue was with
spaces, I then foolishly assumed requests was using urllib.quote which you
then implemented 😅.
Sounds like the proper fix is to use the exact same mechanism as requests
itself and I'm all for it!
|
Avoiding upgrading requests-mock to latest as it's got a bug that affects us: jamielennox/requests-mock#170
Avoiding upgrading requests-mock to latest as it's got a bug that affects us: jamielennox/requests-mock#170
Released as 1.9.2. Apologies for the break, this was not a very smooth release and i'll be more careful in future, |
…sions" This reverts commit 50c6b1f. The issue was coming from a bug in requests-mock that is now fixed in version 1.9.2 of the package so revert introduced workaround and exclude buggy versions of requests-mock in requirements-test. See jamielennox/requests-mock#170
No problem. requests-mock is a really valuable tool. It's great to see this fixed so quickly. |
As part of #158, i took the easy way to quote a string and sent it through urlparse.quote merged as part of #162. However if you already have a quoted string (which you would have to have this work until now) this will double quote it and break.
This is a compatibility break and a general bug - sorry, that's my mistake.
I think the right solution here is to do the same thing requests does, so we should use the same quoting logic rather than the simplistic case.
The text was updated successfully, but these errors were encountered: