-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
GbqConnector should be able to fetch default credentials on Google Compute Engine #13608
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
Changes from 7 commits
9ec2a68
17f4740
9740938
a02b620
fca8003
7f1fd26
d4c7e3d
3479ed8
17dd814
5a2dd15
64134ac
5fd1775
62815e1
cf41e76
ac48104
4cf26aa
6fe392a
6bbdcdc
a65b3f1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -159,7 +159,44 @@ def get_credentials(self): | |
if self.private_key: | ||
return self.get_service_account_credentials() | ||
else: | ||
return self.get_user_account_credentials() | ||
# Try to retrieve Application Default Credentials | ||
credentials = self.get_application_default_credentials() | ||
if not credentials: | ||
credentials = self.get_user_account_credentials() | ||
return credentials | ||
|
||
def get_application_default_credentials(self): | ||
""" | ||
This method tries to retrieve the "default application credentials" | ||
Could be useful for running code on Google Cloud Platform | ||
""" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add a Returns section. Explaining that it will return the credentials or None if not found / error. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. explain the conditions under which this will get the correct credentials |
||
from oauth2client.client import AccessTokenRefreshError | ||
try: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. are these libararies a newer / different version that we are currently importing? |
||
from googleapiclient.discovery import build | ||
from googleapiclient.errors import HttpError | ||
except: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
from apiclient.discovery import build | ||
from apiclient.errors import HttpError | ||
try: | ||
from oauth2client.client import GoogleCredentials | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you can just put all of the imports in 1 try/except block |
||
from oauth2client.client import ApplicationDefaultCredentialsError | ||
except ImportError: | ||
return None | ||
|
||
credentials = None | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you don't need to set credentials, ether you have them or the except has hit and you have returned |
||
try: | ||
credentials = GoogleCredentials.get_application_default() | ||
except ApplicationDefaultCredentialsError: | ||
return None | ||
# Check if the application has rights to the BigQuery project | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. blank line |
||
bigquery_service = build('bigquery', 'v2', credentials=credentials) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Needs to be in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @parthea I've changed the call to the "build" method - should work for both api versions now. |
||
jobs = bigquery_service.jobs() | ||
job_data = {'configuration': {'query': {'query': 'SELECT 1'}}} | ||
try: | ||
jobs.insert(projectId=self.project_id, body=job_data).execute() | ||
except (AccessTokenRefreshError, HttpError, TypeError): | ||
return None | ||
return credentials | ||
|
||
def get_user_account_credentials(self): | ||
from oauth2client.client import OAuth2WebServerFlow | ||
|
@@ -576,10 +613,14 @@ def read_gbq(query, project_id=None, index_col=None, col_order=None, | |
https://developers.google.com/api-client-library/python/apis/bigquery/v2 | ||
|
||
Authentication to the Google BigQuery service is via OAuth 2.0. | ||
By default user account credentials are used. You will be asked to | ||
grant permissions for product name 'pandas GBQ'. It is also posible | ||
to authenticate via service account credentials by using | ||
private_key parameter. | ||
If "private_key" is not provided: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. use Add in a versionadded tag (for the changed one) |
||
By default "application default credentials" are used [new behavior] | ||
If default application credentials are not found or are restrictive - | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. use full sentences. |
||
User account credentials are used. In this case - you will be asked to | ||
grant permissions for product name 'pandas GBQ'. | ||
If "private_key" is provided: | ||
It is also posible to authenticate via service account credentials | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what does this mean? |
||
by using this parameter. | ||
|
||
Parameters | ||
---------- | ||
|
@@ -671,11 +712,14 @@ def to_gbq(dataframe, destination_table, project_id, chunksize=10000, | |
Documentation is available at | ||
https://developers.google.com/api-client-library/python/apis/bigquery/v2 | ||
|
||
Authentication to the Google BigQuery service is via OAuth 2.0. | ||
By default user account credentials are used. You will be asked to | ||
grant permissions for product name 'pandas GBQ'. It is also posible | ||
to authenticate via service account credentials by using | ||
private_key parameter. | ||
If "private_key" is not provided: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same as above |
||
By default "application default credentials" are used [new behavior] | ||
If default application credentials are not found or are restrictive - | ||
User account credentials are used. In this case - you will be asked to | ||
grant permissions for product name 'pandas GBQ'. | ||
If "private_key" is provided: | ||
It is also posible to authenticate via service account credentials | ||
by using this parameter. | ||
|
||
Parameters | ||
---------- | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -80,6 +80,7 @@ def _test_imports(): | |
from apiclient.discovery import build # noqa | ||
from apiclient.errors import HttpError # noqa | ||
|
||
from oauth2client.client import GoogleCredentials # noqa | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Move this inside of |
||
from oauth2client.client import OAuth2WebServerFlow # noqa | ||
from oauth2client.client import AccessTokenRefreshError # noqa | ||
|
||
|
@@ -217,6 +218,12 @@ def test_should_be_able_to_get_results_from_query(self): | |
schema, pages = self.sut.run_query('SELECT 1') | ||
self.assertTrue(pages is not None) | ||
|
||
def test_get_application_default_credentials_should_not_throw_error(self): | ||
from oauth2client.client import GoogleCredentials | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so you need to do skipping based on resource availablity, see the other tests. |
||
credentials = self.sut.get_application_default_credentials() | ||
valid_types = (type(None), GoogleCredentials) | ||
assert isinstance(credentials, valid_types) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. use self.assertTrue and needs to be split for the |
||
|
||
|
||
class TestGBQConnectorServiceAccountKeyPathIntegration(tm.TestCase): | ||
def setUp(self): | ||
|
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.
this is a bit confusing to a reader. pls make several sentences.