CLN: remove deprecated private_key auth logic (#302)

* CLN: remove deprecated private_key auth logic * remove unnecessary deprecation warning for private_key * update docstrings for deprecated private_key arg
googleapis · Dec 12, 2019 · 2897b81 · 2897b81
1 parent 9fb2464
commit 2897b81
Show file tree

Hide file tree

Showing 8 changed files with 114 additions and 308 deletions.
diff --git a/docs/source/changelog.rst b/docs/source/changelog.rst
@@ -1,6 +1,15 @@
 Changelog
 =========
 
+.. _changelog-0.13.0:
+
+0.13.0 / 2019-12-12
+-------------------
+
+- Raise ``NotImplementedError`` when the deprecated ``private_key`` argument
+  is used. (:issue:`301`)
+
+
 .. _changelog-0.12.0:
 
 0.12.0 / 2019-11-25

diff --git a/noxfile.py b/noxfile.py
@@ -51,7 +51,7 @@ def unit(session):
 @nox.session
 def cover(session, python=latest_python):
     session.install("coverage", "pytest-cov")
-    session.run("coverage", "report", "--show-missing", "--fail-under=74")
+    session.run("coverage", "report", "--show-missing", "--fail-under=73")
     session.run("coverage", "erase")
 
 

diff --git a/pandas_gbq/auth.py b/pandas_gbq/auth.py
@@ -1,11 +1,6 @@
 """Private module for fetching Google BigQuery credentials."""
 
-import json
 import logging
-import os
-import os.path
-
-import pandas_gbq.exceptions
 
 logger = logging.getLogger(__name__)
 
@@ -36,7 +31,13 @@ def get_credentials(
     import pydata_google_auth
 
     if private_key:
-        return get_service_account_credentials(private_key)
+        raise NotImplementedError(
+            """The private_key argument is deprecated. Construct a credentials
+object, instead, by using the
+google.oauth2.service_account.Credentials.from_service_account_file or
+google.oauth2.service_account.Credentials.from_service_account_info class
+method from the google-auth package."""
+        )
 
     credentials, default_project_id = pydata_google_auth.default(
         SCOPES,
@@ -50,48 +51,6 @@ def get_credentials(
     return credentials, project_id
 
 
-def get_service_account_credentials(private_key):
-    """DEPRECATED: Load service account credentials from key data or key path."""
-
-    import google.auth.transport.requests
-    from google.oauth2.service_account import Credentials
-
-    is_path = os.path.isfile(private_key)
-
-    try:
-        if is_path:
-            with open(private_key) as f:
-                json_key = json.loads(f.read())
-        else:
-            # ugly hack: 'private_key' field has new lines inside,
-            # they break json parser, but we need to preserve them
-            json_key = json.loads(private_key.replace("\n", "   "))
-            json_key["private_key"] = json_key["private_key"].replace(
-                "   ", "\n"
-            )
-
-        json_key["private_key"] = bytes(json_key["private_key"], "UTF-8")
-        credentials = Credentials.from_service_account_info(json_key)
-        credentials = credentials.with_scopes(SCOPES)
-
-        # Refresh the token before trying to use it.
-        request = google.auth.transport.requests.Request()
-        credentials.refresh(request)
-
-        return credentials, json_key.get("project_id")
-    except (KeyError, ValueError, TypeError, AttributeError):
-        raise pandas_gbq.exceptions.InvalidPrivateKeyFormat(
-            "Detected private_key as {}. ".format(
-                "path" if is_path else "contents"
-            )
-            + "Private key is missing or invalid. It should be service "
-            "account private key JSON (file path or string contents) "
-            'with at least two keys: "client_email" and "private_key". '
-            "Can be obtained from: https://console.developers.google."
-            "com/permissions/serviceaccounts"
-        )
-
-
 def get_credentials_cache(reauth,):
     import pydata_google_auth.cache
 

diff --git a/pandas_gbq/gbq.py b/pandas_gbq/gbq.py
@@ -20,14 +20,6 @@
 BIGQUERY_INSTALLED_VERSION = None
 BIGQUERY_CLIENT_INFO_VERSION = "1.12.0"
 HAS_CLIENT_INFO = False
-SHOW_VERBOSE_DEPRECATION = False
-SHOW_PRIVATE_KEY_DEPRECATION = False
-PRIVATE_KEY_DEPRECATION_MESSAGE = (
-    "private_key is deprecated and will be removed in a future version."
-    "Use the credentials argument instead. See "
-    "https://pandas-gbq.readthedocs.io/en/latest/howto/authentication.html "
-    "for examples on using the credentials argument with service account keys."
-)
 
 try:
     import tqdm  # noqa
@@ -36,7 +28,7 @@
 
 
 def _check_google_client_version():
-    global BIGQUERY_INSTALLED_VERSION, HAS_CLIENT_INFO, SHOW_VERBOSE_DEPRECATION, SHOW_PRIVATE_KEY_DEPRECATION
+    global BIGQUERY_INSTALLED_VERSION, HAS_CLIENT_INFO, SHOW_VERBOSE_DEPRECATION
 
     try:
         import pkg_resources
@@ -74,10 +66,6 @@ def _check_google_client_version():
     SHOW_VERBOSE_DEPRECATION = (
         pandas_installed_version >= pandas_version_wo_verbosity
     )
-    pandas_version_with_credentials_arg = pkg_resources.parse_version("0.24.0")
-    SHOW_PRIVATE_KEY_DEPRECATION = (
-        pandas_installed_version >= pandas_version_with_credentials_arg
-    )
 
 
 def _test_google_api_imports():
@@ -951,27 +939,12 @@ def read_gbq(
         results.
 
         .. versionadded:: 0.12.0
-    verbose : None, deprecated
-        Deprecated in Pandas-GBQ 0.4.0. Use the `logging module
-        to adjust verbosity instead
-        <https://pandas-gbq.readthedocs.io/en/latest/intro.html#logging>`__.
-    private_key : str, deprecated
-        Deprecated in pandas-gbq version 0.8.0. Use the ``credentials``
-        parameter and
-        :func:`google.oauth2.service_account.Credentials.from_service_account_info`
-        or
-        :func:`google.oauth2.service_account.Credentials.from_service_account_file`
-        instead.
-
-        Service account private key in JSON format. Can be file path
-        or string contents. This is useful for remote server
-        authentication (eg. Jupyter/IPython notebook on remote host).
-
     progress_bar_type (Optional[str]):
-        If set, use the `tqdm <https://tqdm.github.io/>`_ library to
+        If set, use the `tqdm <https://tqdm.github.io/>`__ library to
         display a progress bar while the data downloads. Install the
         ``tqdm`` package to use this feature.
         Possible values of ``progress_bar_type`` include:
+
         ``None``
             No progress bar.
         ``'tqdm'``
@@ -983,6 +956,17 @@ def read_gbq(
         ``'tqdm_gui'``
             Use the :func:`tqdm.tqdm_gui` function to display a
             progress bar as a graphical dialog box.
+    verbose : None, deprecated
+        Deprecated in Pandas-GBQ 0.4.0. Use the `logging module
+        to adjust verbosity instead
+        <https://pandas-gbq.readthedocs.io/en/latest/intro.html#logging>`__.
+    private_key : str, deprecated
+        Deprecated in pandas-gbq version 0.8.0. Use the ``credentials``
+        parameter and
+        :func:`google.oauth2.service_account.Credentials.from_service_account_info`
+        or
+        :func:`google.oauth2.service_account.Credentials.from_service_account_file`
+        instead.
 
     Returns
     -------
@@ -1008,11 +992,6 @@ def read_gbq(
             stacklevel=2,
         )
 
-    if private_key is not None and SHOW_PRIVATE_KEY_DEPRECATION:
-        warnings.warn(
-            PRIVATE_KEY_DEPRECATION_MESSAGE, FutureWarning, stacklevel=2
-        )
-
     if dialect not in ("legacy", "standard"):
         raise ValueError("'{0}' is not valid for dialect".format(dialect))
 
@@ -1172,10 +1151,6 @@ def to_gbq(
         or
         :func:`google.oauth2.service_account.Credentials.from_service_account_file`
         instead.
-
-        Service account private key in JSON format. Can be file path
-        or string contents. This is useful for remote server
-        authentication (eg. Jupyter/IPython notebook on remote host).
     """
 
     _test_google_api_imports()
@@ -1190,11 +1165,6 @@ def to_gbq(
             stacklevel=1,
         )
 
-    if private_key is not None and SHOW_PRIVATE_KEY_DEPRECATION:
-        warnings.warn(
-            PRIVATE_KEY_DEPRECATION_MESSAGE, FutureWarning, stacklevel=2
-        )
-
     if if_exists not in ("fail", "replace", "append"):
         raise ValueError("'{0}' is not valid for if_exists".format(if_exists))
 

diff --git a/pandas_gbq/load.py b/pandas_gbq/load.py
@@ -1,6 +1,7 @@
 """Helper methods for loading data into BigQuery"""
 
-import six
+import io
+
 from google.cloud import bigquery
 
 import pandas_gbq.schema
@@ -12,7 +13,7 @@ def encode_chunk(dataframe):
     Args:
       dataframe (pandas.DataFrame): A chunk of a dataframe to encode
     """
-    csv_buffer = six.StringIO()
+    csv_buffer = io.StringIO()
     dataframe.to_csv(
         csv_buffer,
         index=False,
@@ -25,10 +26,8 @@ def encode_chunk(dataframe):
     # Convert to a BytesIO buffer so that unicode text is properly handled.
     # See: https://github.com/pydata/pandas-gbq/issues/106
     body = csv_buffer.getvalue()
-    if isinstance(body, bytes):
-        body = body.decode("utf-8")
     body = body.encode("utf-8")
-    return six.BytesIO(body)
+    return io.BytesIO(body)
 
 
 def encode_chunks(dataframe, chunksize=None):

diff --git a/tests/system/test_auth.py b/tests/system/test_auth.py
@@ -1,5 +1,6 @@
 """System tests for fetching Google BigQuery credentials."""
 
+import os
 from unittest import mock
 
 import pytest
@@ -57,34 +58,11 @@ def _check_if_can_get_correct_default_credentials():
 
 
 def test_should_be_able_to_get_valid_credentials(project_id, private_key_path):
-    credentials, _ = auth.get_credentials(
-        project_id=project_id, private_key=private_key_path
-    )
+    os.environ["GOOGLE_APPLICATION_CREDENTIALS"] = private_key_path
+    credentials, _ = auth.get_credentials(project_id=project_id)
     assert credentials.valid
 
 
-def test_get_service_account_credentials_private_key_path(private_key_path):
-    from google.auth.credentials import Credentials
-
-    credentials, project_id = auth.get_service_account_credentials(
-        private_key_path
-    )
-    assert isinstance(credentials, Credentials)
-    assert _try_credentials(project_id, credentials) is not None
-
-
-def test_get_service_account_credentials_private_key_contents(
-    private_key_contents,
-):
-    from google.auth.credentials import Credentials
-
-    credentials, project_id = auth.get_service_account_credentials(
-        private_key_contents
-    )
-    assert isinstance(credentials, Credentials)
-    assert _try_credentials(project_id, credentials) is not None
-
-
 @pytest.mark.local_auth
 def test_get_credentials_bad_file_returns_user_credentials(
     project_id, monkeypatch

diff --git a/tests/unit/test_auth.py b/tests/unit/test_auth.py
@@ -1,63 +1,23 @@
 # -*- coding: utf-8 -*-
 
 import json
-import os.path
-
-from pandas_gbq import auth
-
 from unittest import mock
 
+import pytest
 
-def test_get_credentials_private_key_contents(monkeypatch):
-    from google.oauth2 import service_account
+from pandas_gbq import auth
 
-    @classmethod
-    def from_service_account_info(cls, key_info):
-        mock_credentials = mock.create_autospec(cls)
-        mock_credentials.with_scopes.return_value = mock_credentials
-        mock_credentials.refresh.return_value = mock_credentials
-        return mock_credentials
 
-    monkeypatch.setattr(
-        service_account.Credentials,
-        "from_service_account_info",
-        from_service_account_info,
-    )
+def test_get_credentials_private_key_raises_notimplementederror(monkeypatch):
     private_key = json.dumps(
         {
             "private_key": "some_key",
             "client_email": "service-account@example.com",
             "project_id": "private-key-project",
         }
     )
-    credentials, project = auth.get_credentials(private_key=private_key)
-
-    assert credentials is not None
-    assert project == "private-key-project"
-
-
-def test_get_credentials_private_key_path(monkeypatch):
-    from google.oauth2 import service_account
-
-    @classmethod
-    def from_service_account_info(cls, key_info):
-        mock_credentials = mock.create_autospec(cls)
-        mock_credentials.with_scopes.return_value = mock_credentials
-        mock_credentials.refresh.return_value = mock_credentials
-        return mock_credentials
-
-    monkeypatch.setattr(
-        service_account.Credentials,
-        "from_service_account_info",
-        from_service_account_info,
-    )
-    private_key = os.path.join(
-        os.path.dirname(__file__), "..", "data", "dummy_key.json"
-    )
-    credentials, project = auth.get_credentials(private_key=private_key)
-
-    assert credentials is not None
-    assert project is None
+    with pytest.raises(NotImplementedError, match="private_key"):
+        auth.get_credentials(private_key=private_key)
 
 
 def test_get_credentials_default_credentials(monkeypatch):
@@ -101,3 +61,12 @@ def mock_default_credentials(scopes=None, request=None):
     credentials, project = auth.get_credentials()
     assert project is None
     assert credentials is mock_user_credentials
+
+
+def test_get_credentials_cache_w_reauth():
+    import pydata_google_auth.cache
+
+    cache = auth.get_credentials_cache(True)
+    assert isinstance(
+        cache, pydata_google_auth.cache.WriteOnlyCredentialsCache
+    )