Skip to content

Conversation

@adamkewley
Copy link

… python3

- Happens because python3 HTTPResponse, when read like a file (e.g. with `json.load`), assumes binary
- see: https://stackoverflow.com/questions/23049767/parsing-http-response-in-python
- Updated test mocks to make unit tests pass
@coveralls
Copy link

Coverage Status

Coverage remained the same at 96.094% when pulling 1df9dbe on adamkewley:master into 33eb832 on toidi:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage remained the same at 96.094% when pulling 1df9dbe on adamkewley:master into 33eb832 on toidi:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 96.094% when pulling 1df9dbe on adamkewley:master into 33eb832 on toidi:master.

@ottomata
Copy link

ottomata commented Jul 3, 2018

I'm trying to use jupyter-enterprise-gateway, which depends on this project, and am also encountering this issue.

class Response(object):
def __init__(self, http_response):
self.data = json.load(http_response)
self.data = json.loads(http_response.read().decode(http_response.headers.get_content_charset('utf-8')))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like how it's implemented in #7 , but there content charset is not taken into account.

Perhaps use codecs module in combination with http_response.headers.get_content_charset('utf-8') ?

@ediskandarov-cur
Copy link
Collaborator

@adamkewley please rebase from master and bump version as well

@lresende
Copy link
Collaborator

This might become obsolete with PR #16 and move to use requests package.

@dimon222
Copy link
Collaborator

This can be probably already closed since this code is no longer present (due to direct call to json() via requests)

@kevin-bates
Copy link
Member

This code is no longer applicable - closing.

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.

7 participants