feat: initializing the new data client #1238

gkevinzheng · 2025-12-03T15:49:01Z

Changes:

Added a new property new_table_data_client (might need to be renamed) in the classic client for the new table data client
Added two hidden kwargs (client_info and disable_background_channel_refresh) to the new data client constructor to support the classic client.

daniel-sanche · 2025-12-03T22:57:19Z

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

            warnings.warn("pool_size no longer supported")
        # set up client info headers for veneer library
-        self.client_info = DEFAULT_CLIENT_INFO
+        self.client_info = kwargs.get("client_info") or DEFAULT_CLIENT_INFO


nit: this can be `kwargs.get("client_info", DEFAULT_CLIENT_INFO)

google/cloud/bigtable/client.py

daniel-sanche · 2025-12-03T23:15:11Z

google/cloud/bigtable/client.py

    _table_data_client = None
    _table_admin_client = None
    _instance_admin_client = None
+    _new_table_data_client = None


We should avoid calling this "new", because someday it will be old

We don't need internal references for both the gapic client and the veneer data client, since the veneer holds a copy of the gapic.

So we can probably remove _table_data_client and _new_table_data_client, and just have a _data_client, that holds the veneer.

Then in def table_data_client(self):, return self._data_client._gapic_client

daniel-sanche · 2025-12-03T23:15:43Z

google/cloud/bigtable/client.py

        return self._instance_admin_client

+    @property
+    def new_table_data_client(self):


it makes sense to make a lazy-loader for this, but I don't think we want to expose this publicly

daniel-sanche · 2025-12-03T23:16:02Z

google/cloud/bigtable/client.py

+        """Getter for the new Data Table API.
+
+        TODO: Replace table_data_client with this implementation
+        when shimming legacy client is finished.


I don't think that's a goal. We should keep returning the gapic client, to keep things consistent (although I wouldn't actually expect many users to use the gapic data cleint direclty)

daniel-sanche · 2025-12-03T23:21:44Z

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

            warnings.warn("pool_size no longer supported")
        # set up client info headers for veneer library
-        self.client_info = DEFAULT_CLIENT_INFO
+        self.client_info = kwargs.get("client_info") or DEFAULT_CLIENT_INFO


add a comment above this. Maybe something like # client_info argument intended for internal use only, to support legacy client shim

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

daniel-sanche · 2025-12-03T23:24:16Z

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

        # set up client info headers for veneer library
-        self.client_info = DEFAULT_CLIENT_INFO
+        self.client_info = kwargs.get("client_info") or DEFAULT_CLIENT_INFO
        self.client_info.client_library_version = self._client_version()


I wonder if we want to set a different user agent for the shim... We'll probably want to know how many people are using the shim, to tell us when it can be removed

Maybe that means we should build our own client_info in the legacy constructor, and only attach this version string if none was passed in?

if "_client_info" in kwargs: # use client_info passed in from legacy client self.client_info = kwargs["_client_info"] else: # set up client info headers for veneer library self.client_info = DEFAULT_CLIENT_INFO self.client_info.client_library_version = self._client_version()

google/cloud/bigtable/client.py

daniel-sanche · 2025-12-03T23:35:28Z

tests/unit/data/_async/test_client.py

+            disable_background_channel_refresh=True,
+        )
+        # should create background tasks for each channel
+        with mock.patch.object(client, "_ping_and_warm_instances", CrossSync.Mock()):


Do you need to mock this? I think that was done in other tests as a crude way of disabling background refresh, but maybe that's not needed with the new argument

daniel-sanche · 2025-12-03T23:43:34Z

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

            warnings.warn("pool_size no longer supported")
        # set up client info headers for veneer library
-        self.client_info = DEFAULT_CLIENT_INFO
+        self.client_info = kwargs.get("client_info") or DEFAULT_CLIENT_INFO


Another thought: We should use a leading underscore for these undocumented kwargs, to make it extra clear that users shouldn't use them if they find them in the code

daniel-sanche · 2025-12-09T20:23:21Z

google/cloud/bigtable/client.py

        return self._instance_admin_client

+    @property
+    def _data_client(self):


nit: having table_data_client and _data_client is a little weird. Since this is just for internal use, maybe we should use _veneer_data_client to be more specific?

daniel-sanche · 2025-12-09T20:26:52Z

google/cloud/bigtable/client.py

+                project=self.project,
+                credentials=self._credentials,
+                client_options=self._client_options,
+                _client_info=self._client_info,


I was thinking we should customize the client_library_version like in the veneer client, so we can specifically see which requests are going through the shim. Maybe `f"{google.cloud.bigtable.version}-shim"?

What do you think? (Maybe ping Mattie?)

daniel-sanche · 2025-12-09T20:28:40Z

google/cloud/bigtable/table.py

+                self.table_id,
+            )
+            if self._instance
+            else None


Why do you need if self._instance? It doesn't seem optional?

I don't think we'd ever want this to be None, right?

daniel-sanche · 2025-12-09T20:32:24Z

google/cloud/bigtable/table.py

+        self._table_impl = (
+            self._instance._client._data_client.get_table(
+                self._instance.instance_id,
+                self.table_id,


This is missing app_profile_id.

And maybe mutation_timeout should be mapped to default_mutate_rows_operation_timeout? Or maybe the timeout can just be passed in with each request. I guess you can leave that part for when you start looking at myutations

_data_client

daniel-sanche

I added a couple suggestions, but let me know if I'm missing context

daniel-sanche · 2025-12-18T00:39:06Z

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

-        self.client_info = DEFAULT_CLIENT_INFO
+
+        # Private argument, for internal use only
+        self._is_legacy_client = bool(kwargs.get("_is_legacy_client", False))


If this is just for customizing the client_version/background refresh, I'd prefer to avoid tracking this extra state.

We're already passing in a client_info object, so I'd say we should set the value in the legacy client before passing it in, and then do something like:

if kwargs.get("_client_info): self.client_info = kwargs.get("_client_info") else: self.client_info = DEFAULT_CLIENT_INFO self.client_info.client_library_version = self._client_version()

daniel-sanche · 2025-12-18T00:42:32Z

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

            not self._channel_refresh_task
            and not self._emulator_host
            and not self._is_closed.is_set()
+            and not self._is_legacy_client


I did like the old version, where there was a flag specifically for disabling background refresh. That could be useful for other purposes too, like testing

Enabling _is_legacy_client could come with other side-effects

feat: initializing the new data client

406ae29

gkevinzheng requested review from a team as code owners December 3, 2025 15:49

blunderbuss-gcf bot assigned vermas2012 Dec 3, 2025

product-auto-label bot added size: l Pull request size is large. api: bigtable Issues related to the googleapis/python-bigtable API. labels Dec 3, 2025

gkevinzheng requested a review from daniel-sanche December 3, 2025 15:49

daniel-sanche reviewed Dec 3, 2025

View reviewed changes

gkevinzheng added 4 commits December 9, 2025 18:49

Addressed code review feedback + test fixes

f1e03e6

Merge branch 'v3_staging' into data-client-init

abc9b51

Added comment

ca1b715

Moved _DEFAULT_BIGTABLE_EMULATOR_CLIENT to _helpers

e534cc2

daniel-sanche reviewed Dec 9, 2025

View reviewed changes

gkevinzheng added 4 commits December 10, 2025 17:04

Added app_profile_id, legacy client shim version, and renamed

0afdbf2

_data_client

linting

39ae646

Added lazy initialization to table object

7f49237

Reverted _veneer_data_table property, fixed tests

d173efe

daniel-sanche reviewed Dec 18, 2025

View reviewed changes

gkevinzheng added 2 commits December 18, 2025 21:53

reverted _is_legacy_client

225df82

linting

7a7e8e3

gkevinzheng added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 19, 2025

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 19, 2025

daniel-sanche approved these changes Dec 19, 2025

View reviewed changes

gkevinzheng merged commit 914006d into v3_staging Dec 19, 2025
6 of 9 checks passed

gkevinzheng deleted the data-client-init branch December 19, 2025 17:04

feat: initializing the new data client #1238

feat: initializing the new data client #1238

Uh oh!

Conversation

gkevinzheng commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

daniel-sanche Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

daniel-sanche Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

daniel-sanche Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

daniel-sanche Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

daniel-sanche Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

daniel-sanche Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

daniel-sanche Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

daniel-sanche Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

daniel-sanche Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

daniel-sanche Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

daniel-sanche Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

daniel-sanche Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

daniel-sanche left a comment

Choose a reason for hiding this comment

Uh oh!

daniel-sanche Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

daniel-sanche Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

gkevinzheng commented Dec 3, 2025 •

edited

Loading

daniel-sanche Dec 3, 2025 •

edited

Loading

daniel-sanche Dec 3, 2025 •

edited

Loading

daniel-sanche Dec 3, 2025 •

edited

Loading

daniel-sanche Dec 3, 2025 •

edited

Loading

daniel-sanche Dec 3, 2025 •

edited

Loading

daniel-sanche Dec 18, 2025 •

edited

Loading

daniel-sanche Dec 18, 2025 •

edited

Loading