8000 Fix issue for python 3.5 and lower, json.loads would not accept bytes by christophebiocca · Pull Request #359 · twilio/twilio-python · GitHub
[go: up one dir, main page]
Skip to content

Fix issue for python 3.5 and lower, json.loads would not accept bytes #359

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Fix issue for python 3.5 and lower, json.loads would not accept bytes #359

wants to merge 1 commit into from

Conversation

christophebiocca
Copy link

The underlying issue seems to have been introduced in d833182.

Fixes #358.

@christophebiocca
Fix issue for python 3.5 and lower where json.loads will not accept b…
0116eff 
…ytes.

The underlying issue seems to have been introduced in d833182.

Fixes #358.
@ilanbiala
Copy link
Contributor

@christophebiocca thanks for the PR! It seems this code doesn't work for non Python 2 versions, so we'll take a look at your PR and see if we can determine the root problem and get a fix out.

@christophebiocca
Copy link
Author
christophebiocca commented Jun 18, 2017 •
edited
Loading

Weird, it's supposed to work correctly on 3.5, which is the one we use. I may have been a little bit too aggressive with the changes I made.

The root of the issue is that json.loads on this line is being given bytes, and all versions of python3 before 3.6 will treat it as an error.

One thing that might help track down the issue is to decide whether response.content is supposed to be bytes or str.

If the former, then the tests are incorrect (they provide strs) and my fix is actually what's needed.

If the latter, then decoding needs to be put back in.

@ilanbiala ilanbiala mentioned this pull request Jun 19, 2017
Merged
@jmctwilio
Copy link
Contributor

Closing in favor of the other PR.

@jmctwilio jmctwilio closed this Jun 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@christophebiocca @ilanbiala @jmctwilio
0