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.

Sign up for GitHub

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

Jump to bottom

Reuse explicit credentials when creating 'database.spanner_api'. #3722

Merged
merged 3 commits into from
Aug 7, 2017
Merged

Reuse explicit credentials when creating 'database.spanner_api'. #3722

merged 3 commits into from
Aug 7, 2017

Conversation

tseaver
Copy link
Contributor

@tseaver tseaver commented Aug 3, 2017

  • Preserves "custom" credentials (existing code worked only with implicit credentials from the environment).
  • Add tests ensuring scopes are set for correctly for all GAX apis (client uses admin scope, which do not grant data access, while database uses data scope, which does not grant admin access).

Closes #3718.

@tseaver tseaver added api: spanner Issues related to the Spanner API. auth labels Aug 3, 2017
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Aug 3, 2017
theacodes
theacodes reviewed Aug 3, 2017
View reviewed changes
spanner/google/cloud/spanner/database.py Outdated
@@ -154,8 +158,13 @@ def ddl_statements(self):
def spanner_api(self):
"""Helper for session-related API calls."""
if self._spanner_api is None:
base_creds = self._instance._client.credentials
scoped = base_creds.with_scopes((SPANNER_DATA_SCOPE,))

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@tseaver
Reuse explicit credentials when creating 'database.spanner_api'.
de8e18c 
- Preserves "custom" credentials (existing code worked only with
  implicit credentials from the environment).

- Add tests ensuring scopes are set for correctly for all GAX apis
  (client uses admin scope, which do not grant data access, while
  database uses data scope, which does not grant admin access).

Closes #3718.
@tseaver tseaver force-pushed the 3718-spanner-propagate-client-creds-to-db branch from 65013ad to de8e18c Compare August 3, 2017 17:24
@theacodes
Copy link
Contributor

Github buried my comment, re-posting:

checked, and not in this case. the base client would've already scoped the credentials and requires_scopes will return false

with_scopes_if_required won't work for this case.

@tseaver
Copy link
Contributor Author

tseaver commented Aug 7, 2017

@jonparrott Who actually implements with_scopes? I can't find a non-raising concrete implementation except in google.auth.app_engine.

@theacodes
Copy link
Contributor

@tseaver http://google-auth.readthedocs.io/en/latest/_modules/google/oauth2/service_account.html#Credentials.with_scopes

@tseaver
Handle scope replacement correctly.
cf5b2df 
'google.auth.credentials.with_scopes_if_required' doesn't replace scopes,
but only adds them if missing.

Addresses:
#3722 (comment).
@tseaver
Copy link
Contributor Author

tseaver commented Aug 7, 2017

@jonparrott See cf5b2df

@tseaver tseaver merged commit 45ee4bd into master Aug 7, 2017
@tseaver tseaver deleted the 3718-spanner-propagate-client-creds-to-db branch August 7, 2017 22:00
landrito pushed a commit to landrito/google-cloud-python that referenced this pull request Aug 21, 2017
@tseaver @landrito
Reuse explicit credentials when creating 'database.spanner_api'. (goo…
f5b7c79 
…gleapis#3722)

- Preserves "custom" credentials (existing code worked only with
  implicit credentials from the environment).

- Add tests ensuring scopes are set for correctly for all GAX apis
  (client uses admin scope, which do not grant data access, while
  database uses data scope, which does not grant admin access).
landrito pushed a commit to landrito/google-cloud-python that referenced this pull request Aug 22, 2017
@tseaver @landrito
Reuse explicit credentials when creating 'database.spanner_api'. (goo…
098eb5f 
…gleapis#3722)

- Preserves "custom" credentials (existing code worked only with
  implicit credentials from the environment).

- Add tests ensuring scopes are set for correctly for all GAX apis
  (client uses admin scope, which do not grant data access, while
  database uses data scope, which does not grant admin access).
landrito pushed a commit to landrito/google-cloud-python that referenced this pull request Aug 22, 2017
@tseaver @landrito
Reuse explicit credentials when creating 'database.spanner_api'. (goo…
a0a8ec5 
…gleapis#3722)

- Preserves "custom" credentials (existing code worked only with
  implicit credentials from the environment).

- Add tests ensuring scopes are set for correctly for all GAX apis
  (client uses admin scope, which do not grant data access, while
  database uses data scope, which does not grant admin access).
@dhermes dhermes mentioned this pull request Aug 24, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@theacodes theacodes theacodes approved these changes

@dhermes dhermes Awaiting requested review from dhermes

Assignees
No one assigned
Labels
api: spanner Issues related to the Spanner API. auth cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tseaver @theacodes @googlebot