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

Fixed some python2->3 string issues #671

Closed
wants to merge 5 commits into from
Closed

Fixed some python2->3 string issues #671

wants to merge 5 commits into from

Conversation

foozlevazquez
Copy link

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

Coverage Status

Coverage remained the same at 100.0% when pulling 59fff73 on foozlevazquez:master into 56f60bb on GoogleCloudPlatform:master.

@@ -171,6 +172,9 @@ def make_exception(response, content, use_json=True):
:rtype: instance of :class:`GCloudError`, or a concrete subclass.
:returns: Exception specific to the error response.
"""
if six.PY3 and isinstance(content, bytes):
content = content.decode('utf-8') # pragma: NO COVER Py3K

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@dhermes
Copy link
Contributor

dhermes commented Feb 20, 2015

@foozlevazquez How did you test that the failures in #653 are passing? Can you add unit tests?

Also, can you git merge --squash into a single commit? For a 4-line change, 5 commits is too noisy.

PS I see you've signed the CLA, @googlebot is useful sometimes!

@dhermes
Copy link
Contributor

dhermes commented Feb 20, 2015

@foozlevazquez Is this the httplib2 reference you are making:

To successfully use http2lib for Python 3, you absolutely must
understand the following sentence:

** THE RESPONSE HEADERS ARE STRINGS, BUT THE CONTENT BODY IS BYTES **

@foozlevazquez
Copy link
Author

@dhermes Regarding #653, I tested it by hand on the test case that I had when I ran into these bugs in the first place. I'll look into writing some tests and squashing the commits as well.

Regarding httplib2, yes.

@dhermes
Copy link
Contributor

dhermes commented Feb 20, 2015

Great! Let me know if you need some help and I can make some suggestions / lend a hand.

@dhermes
Copy link
Contributor

dhermes commented Feb 20, 2015

@foozlevazquez What's up?

@foozlevazquez
Copy link
Author

@dhermes
I'm working on a cleaned resubmittal.

Also, I realized that #653 has a root cause of some python2-3 string crap that is taking place in oauth2client here - So I'm working on a pull-request for that as well.

Essentially I'm having to untangle a set of fixes I made in a hacking frenzy... :)

Thanks for checking.

@dhermes
Copy link
Contributor

dhermes commented Feb 21, 2015

That's great! I can help on the oauth2client side as well, or we can loop in @craigcitro

The people who added Python 3 support there weren't actually running unit tests (eek!) and I tried to fix most of it in:
googleapis/oauth2client#87

@craigcitro
Copy link
Contributor

+1 to fixing any/all issues in oauth2client. @nathanielmanistaatgoogle or @soltanmm is likely to be quicker than me with the code reviews.

@foozlevazquez
Copy link
Author

The origin of this all is some code that simply does the following:

from gcloud import storage

conn = storage.get_connection('myproj')
[x for x in conn.get_all_buckets()]

The example above dies with the infamous oauth2client.client.AccessTokenRefreshError: invalid_grant exception. i.e. #653
But, this code works under Python2.7, which lead me to find that the headers were being composed incorrectly in oauth2client, specifically here.

I've initiated googleapis/oauth2client#136 to fix and test the above.

To fix problems with make_exception and api_request, I've submitted #675.

vchudnov-g pushed a commit that referenced this pull request Sep 20, 2023
* docs: Minor formatting
chore: Update gapic-generator-python to v1.11.5
build: Update rules_python to 0.24.0

PiperOrigin-RevId: 563436317

Source-Link: googleapis/googleapis@42fd37b

Source-Link: googleapis/googleapis-gen@280264c
Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiMjgwMjY0Y2EwMmZiOTMxNmI0MjM3YTk2ZDBhZjFhMjM0M2E4MWE1NiJ9

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

---------

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
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.

Attribute error thrown in gcloud/exceptions.py
5 participants