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

Ensuring response from httplib2 is treated as str in Python 2 and bytes in Python3 #724

Closed

Conversation

craigloftus
Copy link

When using Python 3 httplib2 returns response content as bytes, but the content is str in Python 2. This causes problems when passing the content to json.loads, which requires str always.

Tests were passing previously because they were mocking the response by httplib2 as a str. These tests have been changed to use the b notation, which resolves to str in Python 2 and bytes in Python 3.

This is the follow-up from PR #697.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Mar 13, 2015
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling a66cb5c on craigloftus:python3_support_improvements into d9e2156 on GoogleCloudPlatform:master.

@@ -256,7 +256,7 @@ def api_request(self, method, path, query_params=None,
content_type = response.get('content-type', '')
if not content_type.startswith('application/json'):
raise TypeError('Expected JSON, got %s' % content_type)
return json.loads(content)
return json.loads(content.decode('utf-8'))

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 68fa37f on craigloftus:python3_support_improvements into d668044 on GoogleCloudPlatform:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 4785d98 on craigloftus:python3_support_improvements into d668044 on GoogleCloudPlatform:master.

@dhermes
Copy link
Contributor

dhermes commented Mar 24, 2015

@tseaver I am working on integrating this (and getting regression3 to pass)

Spurred on by #755 and #754

dhermes added a commit to dhermes/google-cloud-python that referenced this pull request Mar 24, 2015
- Incorporates changes from googleapis#724.
- Also requires httplib2 from HEAD since the bytes/unicode
  header issues have not been released on PyPI yet.
dhermes added a commit to dhermes/google-cloud-python that referenced this pull request Mar 24, 2015
- Incorporates changes from googleapis#724.
- Also requires httplib2 from HEAD since the bytes/unicode
  header issues have not been released on PyPI yet.
@dhermes dhermes mentioned this pull request Mar 24, 2015
@dhermes
Copy link
Contributor

dhermes commented Mar 24, 2015

@craigloftus Thanks for doing this work. I have incorporated it into #756 to keep this from bit-rotting too much as we make more updates to master.

Please re-open or comment if you have any issues with this.

@dhermes dhermes closed this Mar 24, 2015
@craigloftus
Copy link
Author

@dhermes No worries. I learnt a lot and look forward to no longer depending on my fork.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants