2879 requests cleanup #2894
2879 requests cleanup #2894
Conversation
There was a problem hiding this comment.
This looks good to me.
@mekarpeles , could you take a quick look at the sponsorship code changes and make sure you are happy with them? I know you are working on this area at the moment.
If this doesn't conflict with any current in-flight work, I am happy to merge this ASAP.
The switch to Requests was only partially done without any error handling. This switches the error handling to use Requests too.
For the remaining one that's needed import using six so that it's compatible with both Python 2 and 3.
tfmorris
force-pushed
the
2879-requests-cleanup
branch
from
66f0969
to
804739c
Compare
|
Rebased to address merge conflict. Please force refresh any checked out branches. |
|
@cdrini I have put this of dev.openlibrary.org for testing |
| @@ -50,7 +49,7 @@ def get_sponsorships_by_contact_id(contact_id, isbn=None): | |||
| data['json'] = json.dumps(data['json']) # flatten the json field as a string | |||
| r = requests.get( | |||
| lending.config_ia_civicrm_api.get('url', ''), | |||
| params=urllib.urlencode(data), | |||
| params=data, | |||
There was a problem hiding this comment.
I'm concerned about removing the urllib.urleencode here, I feel like titles w/ encodings and other cases necessitated this addition and I get the feeling this is going to break things.
There was a problem hiding this comment.
The Python Docs recommend the use of the Requests module over the Standard Library because it has a much cleaner API and because in handles Unicode issues so much better. On both Python 2 and Python 3 Requests favours Unicode over bytes/str and its convince functions just work without the struggles that the Standard Library imposes.
Closes #2879
Removes unnecessary
urlencode()calls and fixes up the import for the remaining one that's needed.Stakeholders
@cdrini for
vendors.py@tabshaikh for the others - as mentioned in the issue, I think this is your code, so please review.@mekarpeles