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

Python 3 error: step2_exchange confusing unicode/bytes #446

Merged
merged 1 commit into from Mar 3, 2016

Conversation

Projects
None yet
4 participants
@happyspace
Contributor

happyspace commented Feb 29, 2016

In python 3, the code received from the browser will be binary instead of a string. Expand the API for oauth_flow.step2_exchange(code) to allow this value to be passed in as is rather than decoding.

for example:
credentials = self.flow.step2_exchange(b'some random code')

test:
tox -e py34 -- tests.test_client:OAuth2WebServerFlowTest.test_exchange_success_binary_code

resolves: #443

@googlebot googlebot added the cla: yes label Feb 29, 2016

@dhermes

This comment has been minimized.

Show comment
Hide comment
@dhermes

dhermes Feb 29, 2016

Contributor

LGTM pending @nathanielmanistaatgoogle comment

Contributor

dhermes commented Feb 29, 2016

LGTM pending @nathanielmanistaatgoogle comment

@nathanielmanistaatgoogle

This comment has been minimized.

Show comment
Hide comment
@nathanielmanistaatgoogle

nathanielmanistaatgoogle Feb 29, 2016

Contributor

Please amend your commit message to conform to these guidelines.

Contributor

nathanielmanistaatgoogle commented Feb 29, 2016

Please amend your commit message to conform to these guidelines.

@happyspace happyspace closed this Mar 1, 2016

@nathanielmanistaatgoogle

This comment has been minimized.

Show comment
Hide comment
@nathanielmanistaatgoogle
Contributor

nathanielmanistaatgoogle commented Mar 1, 2016

Closed?

@happyspace happyspace reopened this Mar 1, 2016

@happyspace

This comment has been minimized.

Show comment
Hide comment
@happyspace

happyspace Mar 1, 2016

Contributor

Sorry I was updating the comment and wiped everything out with a force push. grrr...

Whew are we back? I updated the comment as per guidelines.

Contributor

happyspace commented Mar 1, 2016

Sorry I was updating the comment and wiped everything out with a force push. grrr...

Whew are we back? I updated the comment as per guidelines.

happyspace added a commit to happyspace/oauth2client-1 that referenced this pull request Mar 1, 2016

Update Test for test_exchange_success_binary_code
As per suggestions nathanielmanistaatgoogle, create local constants for variables used in test.
Use _to_bytes() from _helper to convert to bytes.

resolves: googleapis#446
@nathanielmanistaatgoogle

This comment has been minimized.

Show comment
Hide comment
@nathanielmanistaatgoogle

nathanielmanistaatgoogle Mar 1, 2016

Contributor

Please squash commits with each round of code review back-and-forth. Are you sure that your commit message summary line is fifty characters or fewer?

Contributor

nathanielmanistaatgoogle commented Mar 1, 2016

Please squash commits with each round of code review back-and-forth. Are you sure that your commit message summary line is fifty characters or fewer?

happyspace added a commit to happyspace/oauth2client-1 that referenced this pull request Mar 1, 2016

Update Test for test_exchange_success_binary_code
Use self.assertIsNotNone for not None check.

resolves: googleapis#446
Expand API for step2_exchange
In python 3, the code received from the browser will be binary instead of a string. Expand the API for oauth_flow.step2_exchange(code) to allow this value to be passed as is rather than decoding.

for example:
credentials = self.flow.step2_exchange(b'some random code')

test:
tox -e py34  -- tests.test_client:OAuth2WebServerFlowTest.test_exchange_success_binary_code

resolves:  #443, #446

nathanielmanistaatgoogle added a commit that referenced this pull request Mar 3, 2016

Merge pull request #446 from happyspace/master
Python 3 error: step2_exchange confusing unicode/bytes.

@nathanielmanistaatgoogle nathanielmanistaatgoogle merged commit 15c945f into googleapis:master Mar 3, 2016

3 checks passed

cla/google All necessary CLAs are signed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.01%) to 95.266%
Details

@theacodes theacodes referenced this pull request Apr 15, 2016

Merged

Release v2.0.2. #501

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment