Skip to content
This repository was archived by the owner on Feb 24, 2026. It is now read-only.

Accept redirect_uri as arg to flow creating classmethods.#17

Merged
theacodes merged 3 commits intogoogleapis:masterfrom
michelts:redirect_uri_as_argument_to_creation_classmethods
Oct 6, 2017
Merged

Accept redirect_uri as arg to flow creating classmethods.#17
theacodes merged 3 commits intogoogleapis:masterfrom
michelts:redirect_uri_as_argument_to_creation_classmethods

Conversation

@michelts
Copy link
Copy Markdown
Contributor

@michelts michelts commented Oct 5, 2017

The flow.py docstring tells that I could pass redirect_uri as argument to the classmethods from_client_secrets_file or from_config_file, but this thing is broken with the 0.1.1 updates.

I can still set the redirect_uri myself manually, but in this case, the docstring should be updated. I prefer to keep this approach, I mean, keep the redirect_uri as argument to the classmethods ;)

I also updated the tests, let me know if I missed anything.

@googlebot
Copy link
Copy Markdown

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If your company signed a CLA, they designated a Point of Contact who decides which employees are authorized to participate. You may need to contact the Point of Contact for your company and ask to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the project maintainer to go/cla#troubleshoot.
  • In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again.

@michelts
Copy link
Copy Markdown
Contributor Author

michelts commented Oct 5, 2017

I signed it!

@googlebot
Copy link
Copy Markdown

CLAs look good, thanks!

@theacodes
Copy link
Copy Markdown

@michelts Thanks for doing this, but it seems the only actual change needed it to make sure to pass **kwargs here. What do you think?

@michelts
Copy link
Copy Markdown
Contributor Author

michelts commented Oct 6, 2017

I'm not sure.

The from_client_secrets_file method accept kwargs and propagate to from_client_config.

The from_client_config method doesn't propagate the kwargs to the class, but passes the kwargs for google_auth_oauthlib.helpers.session_from_client_config, that follows to requests_oauthlib.OAuth2Session.

Add the redirect_uri to the kwargs and pass it to the class solves the problem, but I'm not sure the requests_oauthlib.OAuth2Session accepts the redirect_uri in kwargs, so I did an approach to keep this behavior.

I can use only kwargs and pop redirect_uri before reach requests_oauthlib.OAuth2Session, if you prefer this way.

Did I miss anything?

@theacodes
Copy link
Copy Markdown

Add the redirect_uri to the kwargs and pass it to the class solves the problem, but I'm not sure the requests_oauthlib.OAuth2Session accepts the redirect_uri in kwargs, so I did an approach to keep this behavior.

It does accept a redirect_uri arg, and the Flow's redirect_uri is just an alias.

@michelts
Copy link
Copy Markdown
Contributor Author

michelts commented Oct 6, 2017

What a shame :)

It is pretty clear reading the redirect_uri property :)

Sorry by that, I will fix the PR now.

@theacodes
Copy link
Copy Markdown

Sorry by that, I will fix the PR now.

Nothing to be sorry about! Thank you for doing this. :)

@theacodes theacodes merged commit 84252c4 into googleapis:master Oct 6, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants