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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: include updates to properties from Google Auth lib #249

Merged
merged 3 commits into from Nov 3, 2022

Conversation

ScruffyProdigy
Copy link
Contributor

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #<issue_number_goes_here> 馃

@ScruffyProdigy ScruffyProdigy requested review from a team as code owners November 1, 2022 16:43
@product-auto-label product-auto-label bot added the size: s Pull request size is small. label Nov 1, 2022
@@ -5,6 +5,6 @@
#
# e.g., if setup.py has "foo >= 1.14.0, < 2.0.0dev",
# Then this file should have foo==1.14.0
google-auth==2.13.0
google-auth==2.14.0
Copy link

Choose a reason for hiding this comment

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

update real deps as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated. It looks like there are some kokoro dependencies which should be updated (they're currently pinned to version 2.11.0), but it looks like this is best updated through some automated process, as other dependencies will need to be updated as well with hash numbers

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess you are referring to https://github.com/googleapis/google-auth-library-python-oauthlib/blob/main/.kokoro/requirements.txt#L165?

@parthea do you know what is this for and if the google-auth version should be updated here manually?

Copy link
Contributor

@parthea parthea Nov 2, 2022

Choose a reason for hiding this comment

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

testing/constraints-3.6.txt should be testing the minimum versions of dependencies as it says at the top of the file. This is correct. The version in testing/constraints-3.6.txt should be google-auth==2.14.0.

The version of google-auth in .kokoro/requirements.txt is used for the release/docs publication and is not related to presubmits/testing. It will be updated automatically as we update dependencies for the release/docs infrastructure.

@ScruffyProdigy ScruffyProdigy changed the title Include updates to properties from Google Auth lib fix: include updates to properties from Google Auth lib Nov 1, 2022
@@ -133,6 +133,8 @@ def credentials_from_session(session, client_config=None):
token_url=client_config.get("token_uri"),
client_id=client_config.get("client_id"),
client_secret=client_config.get("client_secret"),
token_info_url=client_config.get("token_info_url"),
scopes=session.scope,
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this being added?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This ends up passing the token_info_url from the login config to the credential class, so we can have an easier time doing introspection later. It also passes the scopes used to the credential class because it is useful for querying

Copy link
Contributor

Choose a reason for hiding this comment

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

can you elaborate on "useful for querying"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If a user wants to know whether their credentials have the necessary permissions before trying to use them, they will need to know what scopes were used to create the credentials

Copy link

Choose a reason for hiding this comment

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

The token introspection endpoint is used to retrieve the account identifier for gcloud.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry I meant "why is this being added" for the scopes. I get why token_info_url is being added.

@@ -5,6 +5,6 @@
#
# e.g., if setup.py has "foo >= 1.14.0, < 2.0.0dev",
# Then this file should have foo==1.14.0
google-auth==2.13.0
google-auth==2.14.0
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess you are referring to https://github.com/googleapis/google-auth-library-python-oauthlib/blob/main/.kokoro/requirements.txt#L165?

@parthea do you know what is this for and if the google-auth version should be updated here manually?

@parthea parthea removed their request for review November 2, 2022 17:41
@@ -5,6 +5,6 @@
#
# e.g., if setup.py has "foo >= 1.14.0, < 2.0.0dev",
# Then this file should have foo==1.14.0
google-auth==2.13.0
google-auth==2.14.0
Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM

@sai-sunder-s sai-sunder-s added the owlbot:run Add this label to trigger the Owlbot post processor. label Nov 3, 2022
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Nov 3, 2022
@sai-sunder-s sai-sunder-s merged commit 58becac into googleapis:main Nov 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: s Pull request size is small.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants