-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Switch from oauth2client to google-auth #2726
Conversation
theacodes
commented
Nov 11, 2016
•
edited
Loading
edited
- Removes all use of oauth2client from every package and tests.
- Updates core to use google-auth's default credentials, project ID, and scoping logic.
- Updates bigtable to use google-auth's scoping logic.
4563140
to
2581686
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really great changes! Why do we don't merge this?
import grpc | ||
except ImportError: # pragma: NO COVER | ||
grpc = None | ||
import six | ||
from six.moves import http_client | ||
from six.moves import configparser | ||
|
||
from google.cloud.environment_vars import PROJECT |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
def _determine_default_project(project=None): | ||
"""Determine default project ID explicitly or implicitly as fall-back. | ||
|
||
In implicit case, supports three environments. In order of precedence, the | ||
implicit environments are: | ||
|
||
* GOOGLE_CLOUD_PROJECT environment variable | ||
* GOOGLE_CLOUD_PROJECT and GCLOUD_PROJECT environment variable |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
if project is None: | ||
project = _compute_engine_id() | ||
|
||
_, project = google.auth.default() |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
json_credentials_path) | ||
kwargs['credentials'] = credentials | ||
return cls(*args, **kwargs) | ||
|
||
@classmethod | ||
def from_service_account_p12(cls, client_email, private_key_path, |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
@@ -84,7 +85,8 @@ def get_credentials(): | |||
:returns: A new credentials instance corresponding to the implicit | |||
environment. | |||
""" | |||
return client.GoogleCredentials.get_application_default() | |||
credentials, _ = google.auth.default() |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
@@ -4,6 +4,7 @@ envlist = | |||
|
|||
[testing] | |||
deps = | |||
mock |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
returned_project = self._call_fut(project) | ||
|
||
return returned_project, _callers | ||
def test(self): |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
def test(self): | ||
with mock.patch('google.auth.default', autospec=True) as default: | ||
default.return_value = ( | ||
mock.sentinel.credentials, mock.sentinel.project) |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
project = self._call_fut(mock.sentinel.project) | ||
|
||
self.assertEqual(project, mock.sentinel.project) | ||
self.assertFalse(default.called) |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
conn = self._make_one(credentials) | ||
self.assertIs(conn.credentials, credentials) | ||
self.assertIs(conn.credentials, mock.sentinel.credentials) |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
try: | ||
import google_auth_httplib2 | ||
|
||
try: # pragma: NO COVER |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
with _Monkey(MUT, grpc=grpc_mod, | ||
MetadataPlugin=mock_plugin): | ||
|
||
grpc_patch = mock.patch.object(MUT, 'grpc', new=grpc_mod) |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
request_patch = mock.patch('google_auth_httplib2.Request') | ||
plugin_patch = mock.patch.object( | ||
MUT, 'AuthMetadataPlugin', create=True) | ||
with grpc_patch, request_patch as request_mock, plugin_patch as plugin: |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
@@ -30,58 +30,9 @@ | |||
def get_credentials(): | |||
"""Gets credentials implicitly from the current environment. | |||
|
|||
.. note:: |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
import grpc | ||
except ImportError: # pragma: NO COVER | ||
from google.auth.transport.grpc import AuthMetadataPlugin |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
4820062
to
46f83da
Compare
def create_scoped(self, scope): | ||
self._scopes = scope | ||
return self | ||
pass |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
``$ gcloud beta auth application-default login`` | ||
* Google App Engine application ID | ||
* Google Compute Engine project ID (from metadata server) | ||
See :func:`google.auth.default` for details on how the default project |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
custom_metadata_plugin = MetadataPlugin(credentials) | ||
http = httplib2.Http() | ||
custom_metadata_plugin = AuthMetadataPlugin( | ||
credentials, google_auth_httplib2.Request(http=http)) |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
@@ -90,7 +90,7 @@ def _httplib2_debug_level(http_request, level, http=None): | |||
old_level = httplib2.debuglevel | |||
http_levels = {} | |||
httplib2.debuglevel = level | |||
if http is not None: | |||
if http is not None and hasattr(http, 'connections'): |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
@@ -52,8 +52,9 @@ | |||
REQUIREMENTS = [ | |||
'httplib2 >= 0.9.1', | |||
'googleapis-common-protos >= 1.3.4', | |||
'oauth2client >= 3.0.0, < 4.0.0dev', |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
project = self._call_fut(mock.sentinel.project) | ||
|
||
self.assertEqual(project, mock.sentinel.project) | ||
self.assertFalse(default.called) |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dhermes I think I addressed all of the comments - let me know if there's anything left.
Do not merge remains until I get all system tests to pass.
def create_scoped(self, scope): | ||
self._scopes = scope | ||
return self | ||
pass |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
``$ gcloud beta auth application-default login`` | ||
* Google App Engine application ID | ||
* Google Compute Engine project ID (from metadata server) | ||
See :func:`google.auth.default` for details on how the default project |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
@@ -90,7 +90,7 @@ def _httplib2_debug_level(http_request, level, http=None): | |||
old_level = httplib2.debuglevel | |||
http_levels = {} | |||
httplib2.debuglevel = level | |||
if http is not None: | |||
if http is not None and hasattr(http, 'connections'): |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Unit tests all pass on Circle. Going to try to run through the system tests locally now. |
bigquery, bigtable, and datastore are passing. |
language, logging, monitoring, pubsub, speech, and storage are passing. Translate seems to be failing for unrelated reasons?
That's everything. All seems good? |
@jonparrott Your |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving with malice
Merging with ill intent. |
…kground-update) There is no specific reason to fix version and to manage this dependency differently (get last version), and it solved the issue below: for recent image push I have this error with gcloud 0.21.0: Traceback (most recent call last): File "./src/orchestrator/launch_backgroundupdate.py", line 8, in <module> init_gcloud_log(GCLOUD_PROJECT, u'background_update', IS_TEST_ENV) File "/usr/local/lib/python2.7/site-packages/common/log.py", line 14, in init_gcloud_log handler = CloudLoggingHandler(client, logger_name) File "/usr/local/lib/python2.7/site-packages/google/cloud/logging/handlers/handlers.py", line 80, in __init__ self.transport = transport(client, name) File "/usr/local/lib/python2.7/site-packages/google/cloud/logging/handlers/transports/background_thread.py", line 154, in __init__ http = client._connection.credentials.authorize(http) AttributeError: 'Credentials' object has no attribute 'authorize' It seems related to change: googleapis/google-cloud-python#2726 Upgrade to 0.21.1 fix the issue.
* Removes all use of oauth2client from every package and tests. * Updates core to use google-auth's default credentials, project ID, and scoping logic. * Updates bigtable to use google-auth's scoping logic.
* Removes all use of oauth2client from every package and tests. * Updates core to use google-auth's default credentials, project ID, and scoping logic. * Updates bigtable to use google-auth's scoping logic.
* Removes all use of oauth2client from every package and tests. * Updates core to use google-auth's default credentials, project ID, and scoping logic. * Updates bigtable to use google-auth's scoping logic.
* Removes all use of oauth2client from every package and tests. * Updates core to use google-auth's default credentials, project ID, and scoping logic. * Updates bigtable to use google-auth's scoping logic.
* Removes all use of oauth2client from every package and tests. * Updates core to use google-auth's default credentials, project ID, and scoping logic. * Updates bigtable to use google-auth's scoping logic.
* Removes all use of oauth2client from every package and tests. * Updates core to use google-auth's default credentials, project ID, and scoping logic. * Updates bigtable to use google-auth's scoping logic.
* Removes all use of oauth2client from every package and tests. * Updates core to use google-auth's default credentials, project ID, and scoping logic. * Updates bigtable to use google-auth's scoping logic.
* Removes all use of oauth2client from every package and tests. * Updates core to use google-auth's default credentials, project ID, and scoping logic. * Updates bigtable to use google-auth's scoping logic.