Repository: GoogleCloudPlatform/google-auth-library-python
Pull request #227 (2017-12-07 19:46:27)

Check for project_id envvar before warning
Resolves GoogleCloudPlatform/google-auth-library-python#221

jonparrott: This kind of indicates to me that we should remove the individual warnings and just warn in default()?
JacobHayes: Agreed. The messages were _just_ different enough that I didn't tackle that originally. Would you prefer to make the message the same (ie: not reporting where the `credentials` came from) or something like adding a name to [`checkers`](https://github.com/JacobHayes/google-auth-library-python/blob/f42d53519b6ecf2a4ce235f3c04e34e54b6fa9da/google/auth/_default.py#L281) to fill into the message? Ex:

```python
    checkers = {  # or tuple of two-tuples
        os.environ.get(environment_vars.CREDENTIALS): _get_explicit_environ_credentials,
        'Cloud SDK Credentials': _get_gcloud_sdk_credentials,
        'App Engine metadata service': _get_gae_credentials,
        'Compute Engine metadata service': lambda: _get_gce_credentials(request),
    }
```
jonparrott: > prefer to make the message the same (ie: not reporting where the credentials came from)

That would be fine. :)
JacobHayes: Any ideas on fixing the coverage test?
jonparrott: Write a test that mocks all the checkers to return no project?

Patch:
diff --git a/google/auth/_default.py b/google/auth/_default.py
index 5410a4c..d63dcee 100644
--- a/google/auth/_default.py
+++ b/google/auth/_default.py
@@ -129,12 +129,6 @@ def _get_gcloud_sdk_credentials():
     if not project_id:
         project_id = _cloud_sdk.get_project_id()
 
-    if not project_id:
-        _LOGGER.warning(
-            'No project ID could be determined from the Cloud SDK '
-            'configuration. Consider running `gcloud config set project` or '
-            'setting the %s environment variable', environment_vars.PROJECT)
-
     return credentials, project_id
 
 
@@ -147,12 +141,6 @@ def _get_explicit_environ_credentials():
         credentials, project_id = _load_credentials_from_file(
             os.environ[environment_vars.CREDENTIALS])
 
-        if not project_id:
-            _LOGGER.warning(
-                'No project ID could be determined from the credentials at %s '
-                'Consider setting the %s environment variable',
-                environment_vars.CREDENTIALS, environment_vars.PROJECT)
-
         return credentials, project_id
 
     else:
@@ -188,10 +176,6 @@ def _get_gce_credentials(request=None):
         try:
             project_id = _metadata.get_project_id(request=request)
         except exceptions.TransportError:
-            _LOGGER.warning(
-                'No project ID could be determined from the Compute Engine '
-                'metadata service. Consider setting the %s environment '
-                'variable.', environment_vars.PROJECT)
             project_id = None
 
         return compute_engine.Credentials(), project_id
@@ -287,6 +271,13 @@ def default(scopes=None, request=None):
         credentials, project_id = checker()
         if credentials is not None:
             credentials = with_scopes_if_required(credentials, scopes)
-            return credentials, explicit_project_id or project_id
+            effective_project_id = explicit_project_id or project_id
+            if not effective_project_id:
+                _LOGGER.warning(
+                    'No project ID could be determined. Consider running '
+                    '`gcloud config set project` or setting the %s '
+                    'environment variable',
+                    environment_vars.PROJECT)
+            return credentials, effective_project_id
 
     raise exceptions.DefaultCredentialsError(_HELP_MESSAGE)
diff --git a/google/auth/credentials.py b/google/auth/credentials.py
index db2869c..64b3338 100644
--- a/google/auth/credentials.py
+++ b/google/auth/credentials.py
@@ -209,6 +209,9 @@ class ReadOnlyScoped(object):
         .. warning: This method is not guaranteed to be accurate if the
             credentials are :attr:`~Credentials.invalid`.
 
+        Args:
+            scopes (Sequence[str]): The list of scopes to check.
+
         Returns:
             bool: True if the credentials have the given scopes.
         """
@@ -248,7 +251,8 @@ class Scoped(ReadOnlyScoped):
         """Create a copy of these credentials with the specified scopes.
 
         Args:
-            scopes (Sequence[str]): The list of scopes to request.
+            scopes (Sequence[str]): The list of scopes to attach to the
+                current credentials.
 
         Raises:
             NotImplementedError: If the credentials' scopes can not be changed.


Test patch:
diff --git a/tests/test__default.py b/tests/test__default.py
index 2df8a44..68c4fb0 100644
--- a/tests/test__default.py
+++ b/tests/test__default.py
@@ -301,6 +301,28 @@ def test_default_explict_legacy_project_id(unused_get, monkeypatch):
         mock.sentinel.credentials, 'explicit-env')
 
 
+@mock.patch(
+    'logging.Logger.warning',
+    autospec=True)
+@mock.patch(
+    'google.auth._default._get_explicit_environ_credentials',
+    return_value=(mock.sentinel.credentials, None), autospec=True)
+@mock.patch(
+    'google.auth._default._get_gcloud_sdk_credentials',
+    return_value=(mock.sentinel.credentials, None), autospec=True)
+@mock.patch(
+    'google.auth._default._get_gae_credentials',
+    return_value=(mock.sentinel.credentials, None), autospec=True)
+@mock.patch(
+    'google.auth._default._get_gce_credentials',
+    return_value=(mock.sentinel.credentials, None), autospec=True)
+def test_default_without_project_id(
+        unused_gce, unused_gae, unused_sdk, unused_explicit, logger_warning):
+    assert _default.default() == (
+        mock.sentinel.credentials, None)
+    logger_warning.assert_called_with(mock.ANY, mock.ANY, mock.ANY)
+
+
 @mock.patch(
     'google.auth._default._get_explicit_environ_credentials',
     return_value=(None, None), autospec=True)
@@ -324,7 +346,7 @@ def test_default_fail(unused_gce, unused_gae, unused_sdk, unused_explicit):
     autospec=True)
 @mock.patch(
     'google.auth.credentials.with_scopes_if_required', autospec=True)
-def test_default_scoped(with_scopes, get):
+def test_default_scoped(with_scopes, unused_get):
     scopes = ['one', 'two']
 
     credentials, project_id = _default.default(scopes=scopes)