reverted _is_legacy_client

Author: gkevinzheng

google/cloud/bigtable/client.py

@@ -27,6 +27,7 @@
 * a :class:`~google.cloud.bigtable.table.Table` owns a
   :class:`~google.cloud.bigtable.row.Row` (and all the cells in the row)
 """
+import copy
 import os
 import warnings
 import grpc  # type: ignore
@@ -364,12 +365,14 @@ def instance_admin_client(self):
     def _veneer_data_client(self):
         """Getter for the new Data Table API."""
         if self._table_data_client is None:
+            client_info = copy.copy(self._client_info)
+            client_info.client_library_version = f"{bigtable.__version__}-data-shim"
             self._table_data_client = BigtableDataClient(
                 project=self.project,
                 credentials=self._credentials,
                 client_options=self._client_options,
-                _client_info=self._client_info,
-                _is_legacy_client=True,
+                _client_info=client_info,
+                _disable_background_refresh=True,
             )
         return self._table_data_client
 

google/cloud/bigtable/data/_async/client.py

@@ -185,13 +185,13 @@ def __init__(
         if "pool_size" in kwargs:
             warnings.warn("pool_size no longer supported")
 
-        # Private argument, for internal use only
-        self._is_legacy_client = bool(kwargs.get("_is_legacy_client", False))
-
         # set up client info headers for veneer library. _client_info is for internal use only,
         # for the legacy client shim.
-        self.client_info = kwargs.get("_client_info", DEFAULT_CLIENT_INFO)
-        self.client_info.client_library_version = self._client_version()
+        if kwargs.get("_client_info"):
+            self.client_info = kwargs["_client_info"]
+        else:
+            self.client_info = DEFAULT_CLIENT_INFO
+            self.client_info.client_library_version = self._client_version()
 
         # parse client options
         if type(client_options) is dict:
@@ -242,6 +242,8 @@ def __init__(
                 "is the default."
             )
         self._is_closed = CrossSync.Event()
+        # Private argument, for internal use only
+        self._disable_background_refresh = bool(kwargs.get("_disable_background_refresh", False))
         self.transport = cast(TransportType, self._gapic_client.transport)
         # keep track of active instances to for warmup on channel refresh
         self._active_instances: Set[_WarmedInstanceKey] = set()
@@ -306,13 +308,12 @@ def api_endpoint(self) -> str:
         """
         return self._gapic_client.api_endpoint
 
-    def _client_version(self) -> str:
+    @staticmethod
+    def _client_version() -> str:
         """
         Helper function to return the client version string for this client
         """
         version_str = f"{google.cloud.bigtable.__version__}-data"
-        if self._is_legacy_client:
-            version_str += "-shim"
         if CrossSync.is_async:
             version_str += "-async"
         return version_str
@@ -336,7 +337,7 @@ def _start_background_channel_refresh(self) -> None:
             not self._channel_refresh_task
             and not self._emulator_host
             and not self._is_closed.is_set()
-            and not self._is_legacy_client
+            and not self._disable_background_refresh
         ):
             # raise error if not in an event loop in async client
             CrossSync.verify_async_event_loop()

google/cloud/bigtable/data/_sync_autogen/client.py

@@ -127,9 +127,11 @@ def __init__(
         """
         if "pool_size" in kwargs:
             warnings.warn("pool_size no longer supported")
-        self._is_legacy_client = bool(kwargs.get("_is_legacy_client", False))
-        self.client_info = kwargs.get("_client_info", DEFAULT_CLIENT_INFO)
-        self.client_info.client_library_version = self._client_version()
+        if kwargs.get("_client_info"):
+            self.client_info = kwargs["_client_info"]
+        else:
+            self.client_info = DEFAULT_CLIENT_INFO
+            self.client_info.client_library_version = self._client_version()
         if type(client_options) is dict:
             client_options = client_options_lib.from_dict(client_options)
         client_options = cast(
@@ -169,6 +171,9 @@ def __init__(
                 f"The configured universe domain ({self.universe_domain}) does not match the universe domain found in the credentials ({self._credentials.universe_domain}). If you haven't configured the universe domain explicitly, `googleapis.com` is the default."
             )
         self._is_closed = CrossSync._Sync_Impl.Event()
+        self._disable_background_refresh = bool(
+            kwargs.get("_disable_background_refresh", False)
+        )
         self.transport = cast(TransportType, self._gapic_client.transport)
         self._active_instances: Set[_WarmedInstanceKey] = set()
         self._instance_owners: dict[_WarmedInstanceKey, Set[int]] = {}
@@ -224,11 +229,10 @@ def api_endpoint(self) -> str:
             str: The API endpoint used by the client instance."""
         return self._gapic_client.api_endpoint
 
-    def _client_version(self) -> str:
+    @staticmethod
+    def _client_version() -> str:
         """Helper function to return the client version string for this client"""
         version_str = f"{google.cloud.bigtable.__version__}-data"
-        if self._is_legacy_client:
-            version_str += "-shim"
         return version_str
 
     def _start_background_channel_refresh(self) -> None:
@@ -240,7 +244,7 @@ def _start_background_channel_refresh(self) -> None:
             not self._channel_refresh_task
             and (not self._emulator_host)
             and (not self._is_closed.is_set())
-            and (not self._is_legacy_client)
+            and (not self._disable_background_refresh)
         ):
             CrossSync._Sync_Impl.verify_async_event_loop()
             self._channel_refresh_task = CrossSync._Sync_Impl.create_task(

tests/unit/data/_async/test_client.py

@@ -181,7 +181,6 @@ async def test_ctor_legacy_client(self):
         from google.api_core import client_options as client_options_lib
         from google.api_core.gapic_v1.client_info import ClientInfo
         from google.cloud.bigtable import __version__ as bigtable_version
-        import copy
 
         project = "project-id"
         credentials = AnonymousCredentials()
@@ -192,14 +191,17 @@ async def test_ctor_legacy_client(self):
             CrossSync.GapicClient, "__init__"
         ) as bigtable_client_init:
             try:
-                self._make_client(
+                client = self._make_client(
                     project=project,
                     credentials=credentials,
                     client_options=options_parsed,
                     use_emulator=False,
                     _client_info=client_info,
-                    _is_legacy_client=True,
+                    _disable_background_refresh=True,
                 )
+
+                assert client._disable_background_refresh
+                assert client.client_info is client_info
             except TypeError:
                 pass
 
@@ -209,21 +211,6 @@ async def test_ctor_legacy_client(self):
             assert kwargs["credentials"] == credentials
             assert kwargs["client_options"] == options_parsed
 
-            expected_client_info = copy.copy(client_info)
-            expected_client_info.client_library_version = (
-                f"{bigtable_version}-data-shim"
-                if not CrossSync.is_async
-                else f"{bigtable_version}-data-shim-async"
-            )
-            assert (
-                kwargs["client_info"].to_user_agent()
-                == expected_client_info.to_user_agent()
-            )
-            assert (
-                kwargs["client_info"].to_grpc_metadata()
-                == expected_client_info.to_grpc_metadata()
-            )
-
     @CrossSync.pytest
     async def test_ctor_dict_options(self):
         from google.api_core.client_options import ClientOptions
@@ -316,10 +303,10 @@ async def test__start_background_channel_refresh(self):
             await client.close()
 
     @CrossSync.pytest
-    async def test__start_background_channel_refresh_legacy_client(self):
+    async def test__start_background_channel_refresh_disable_background_refresh(self):
         client = self._make_client(
             project="project-id",
-            _is_legacy_client=True,
+            _disable_background_refresh=True,
         )
         # should create background tasks for each channel
         with mock.patch.object(

tests/unit/data/_sync_autogen/test_client.py

@@ -144,8 +144,6 @@ def test_ctor_super_inits(self):
     def test_ctor_legacy_client(self):
         from google.api_core import client_options as client_options_lib
         from google.api_core.gapic_v1.client_info import ClientInfo
-        from google.cloud.bigtable import __version__ as bigtable_version
-        import copy
 
         project = "project-id"
         credentials = AnonymousCredentials()
@@ -156,34 +154,22 @@ def test_ctor_legacy_client(self):
             CrossSync._Sync_Impl.GapicClient, "__init__"
         ) as bigtable_client_init:
             try:
-                self._make_client(
+                client = self._make_client(
                     project=project,
                     credentials=credentials,
                     client_options=options_parsed,
                     use_emulator=False,
                     _client_info=client_info,
-                    _is_legacy_client=True,
+                    _disable_background_refresh=True,
                 )
+                assert client._disable_background_refresh
+                assert client.client_info is client_info
             except TypeError:
                 pass
             assert bigtable_client_init.call_count == 1
             kwargs = bigtable_client_init.call_args[1]
             assert kwargs["credentials"] == credentials
             assert kwargs["client_options"] == options_parsed
-            expected_client_info = copy.copy(client_info)
-            expected_client_info.client_library_version = (
-                f"{bigtable_version}-data-shim"
-                if not CrossSync._Sync_Impl.is_async
-                else f"{bigtable_version}-data-shim-async"
-            )
-            assert (
-                kwargs["client_info"].to_user_agent()
-                == expected_client_info.to_user_agent()
-            )
-            assert (
-                kwargs["client_info"].to_grpc_metadata()
-                == expected_client_info.to_grpc_metadata()
-            )
 
     def test_ctor_dict_options(self):
         from google.api_core.client_options import ClientOptions
@@ -257,8 +243,10 @@ def test__start_background_channel_refresh(self):
             assert ping_and_warm.call_count == 1
             client.close()
 
-    def test__start_background_channel_refresh_legacy_client(self):
-        client = self._make_client(project="project-id", _is_legacy_client=True)
+    def test__start_background_channel_refresh_disable_background_refresh(self):
+        client = self._make_client(
+            project="project-id", _disable_background_refresh=True
+        )
         with mock.patch.object(
             client, "_ping_and_warm_instances", CrossSync._Sync_Impl.Mock()
         ) as ping_and_warm:

tests/unit/v2_client/test_client.py

@@ -391,32 +391,44 @@ def test_client_project_path():
     assert client.project_path == project_name
 
 
-def test_client_data_client_not_initialized():
+def test_client_veneer_data_client_not_initialized():
     from google.cloud.bigtable.data import BigtableDataClient
+    from google.cloud.bigtable import __version__
 
     credentials = _make_credentials()
     client = _make_client(project=PROJECT, credentials=credentials)
 
-    veneer_data_client = client._veneer_data_client
-    assert isinstance(veneer_data_client, BigtableDataClient)
-    assert client._veneer_data_client is veneer_data_client
-    assert client._veneer_data_client._is_legacy_client
+    with mock.patch("copy.copy") as copy_mock:
+        data_client = client._veneer_data_client
+
+        assert isinstance(data_client, BigtableDataClient)
+        assert client._table_data_client is data_client
+
+    assert client._table_data_client._disable_background_refresh
+    assert client._table_data_client.client_info.client_library_version == f"{__version__}-data-shim"
+    copy_mock.assert_called_once_with(client._client_info)
 
 
 def test_client_veneer_data_client_not_initialized_w_client_info():
     from google.api_core.gapic_v1.client_info import ClientInfo
+    from google.cloud.bigtable import __version__
 
     credentials = _make_credentials()
     client_info = ClientInfo(gapic_version="1.2.3", user_agent="test-client-")
     client = _make_client(
         project=PROJECT, credentials=credentials, client_info=client_info
     )
-    data_client = client._veneer_data_client
+
+    with mock.patch("copy.copy") as copy_mock:
+        data_client = client._veneer_data_client
+
+        assert client._table_data_client is data_client
 
     assert client._client_info is client_info
-    assert client._veneer_data_client is data_client
-    assert client._veneer_data_client.client_info is client_info
-    assert client._veneer_data_client._is_legacy_client
+    assert client._table_data_client.client_info is copy_mock.return_value
+    assert client._table_data_client._disable_background_refresh
+    assert client._table_data_client.client_info.client_library_version == f"{__version__}-data-shim"
+    copy_mock.assert_called_once_with(client_info)
 
 
 def test_client_veneer_data_client_not_initialized_w_client_options():
@@ -429,9 +441,9 @@ def test_client_veneer_data_client_not_initialized_w_client_options():
     )
 
     data_client = client._veneer_data_client
-    assert client._veneer_data_client is data_client
-    assert client._veneer_data_client._is_legacy_client
-    assert client._veneer_data_client._gapic_client._client_options == client_options
+    assert client._table_data_client is data_client
+    assert client._table_data_client._disable_background_refresh
+    assert client._table_data_client._gapic_client._client_options == client_options
 
 
 def test_client_veneer_data_client_initialized():
@@ -462,11 +474,28 @@ def test_client_data_gapic_client_not_initialized_w_client_info():
         project=PROJECT, credentials=credentials, client_info=client_info
     )
 
-    table_data_client = client.table_data_client
+    mock_gapic_client = mock.MagicMock(spec=BigtableClient)
+    mock_gapic_client.universe_domain = BigtableClient._DEFAULT_UNIVERSE
+
+    with mock.patch(
+        "google.cloud.bigtable.data._sync_autogen.client.GapicClient",
+        return_value=mock_gapic_client,
+    ) as gapic_mock:
+        with mock.patch("copy.copy") as copy_mock:
+            table_data_client = client.table_data_client
+
     assert isinstance(table_data_client, BigtableClient)
     assert client._client_info is client_info
     assert client._table_data_client._gapic_client is table_data_client
 
+    copy_mock.assert_called_once_with(client._client_info)
+    gapic_mock.assert_called_once_with(
+        client_info=copy_mock.return_value,
+        credentials=mock.ANY,
+        transport=mock.ANY,
+        client_options=mock.ANY,
+    )
+
 
 def test_client_data_gapic_client_not_initialized_w_client_options():
     from google.api_core.client_options import ClientOptions
@@ -484,13 +513,13 @@ def test_client_data_gapic_client_not_initialized_w_client_options():
     with mock.patch(
         "google.cloud.bigtable.data._sync_autogen.client.GapicClient",
         return_value=mock_gapic_client,
-    ) as mocked:
+    ) as gapic_mock:
         table_data_client = client.table_data_client
 
     assert client._table_data_client._gapic_client is table_data_client
 
-    mocked.assert_called_once_with(
-        client_info=client._client_info,
+    gapic_mock.assert_called_once_with(
+        client_info=mock.ANY,
         credentials=mock.ANY,
         transport=mock.ANY,
         client_options=client_options,