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

Another Python35 json fix #601

Merged
merged 1 commit into from
Nov 29, 2019
Merged

Conversation

mediaminister
Copy link
Collaborator

No description provided.

@mediaminister mediaminister merged commit a5a9155 into add-ons:master Nov 29, 2019
@dagwieers dagwieers added the bug Something isn't working label Nov 29, 2019
@dagwieers dagwieers added this to the v2.3.0 milestone Nov 29, 2019
@dagwieers
Copy link
Collaborator

So I would like to get rid of Python 3.5-specific changes when we stop supporting Python 3.5.
Not sure what the best way is to get reminded of this.

@mediaminister
Copy link
Collaborator Author

mediaminister commented Nov 29, 2019

This change is not really Python 3.5 specific, I just pass unicode text into json.loads and this approach happens to work for all Python 3.x versions.

A wrapper function that takes an url and outputs json can simplify maintenance in this specific case. Using the requests module is also an option. ;-)

@dagwieers
Copy link
Collaborator

urllib3 is a lot faster, but still not nearly as efficient as it used to be. Regardless of the ease it brings. However, there's a lot to say to make a separate module for all HTTP-related functionality (incl. cookie-handling and JSON parsing). Or maybe use a more efficiënt Requests alternative?

For #385 this would be very convenient too :-)

@dagwieers
Copy link
Collaborator

This change is not really Python 3.5 specific, I just pass unicode text into json.loads and this approach happens to work for all Python 3.x versions.

My only concern here was that rather than reading from a handle, this passes the content around, so not as efficiënt as before. I guess when we get rid of Python 2 and Python 3.5 we can clean this up.

@mediaminister mediaminister deleted the python35 branch December 7, 2019 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants