feat(bigtable): pass user agent, tests and start timeout env variable by mutianf · Pull Request #18686 · googleapis/google-cloud-python

mutianf · 2026-10-13T19:56:18Z

Pass Python client version to user agent.
add a start timeout env variable.
add tests.

_{Stack created with GitHub Stacks CLI • Give Feedback 💬}

gemini-code-assist

Code Review

This pull request adds a --caller-user-agent flag containing the Python client version to the accelerator flags in both the async and sync Bigtable data clients, along with corresponding unit tests. The feedback recommends importing google.cloud.bigtable locally within the configuration method to avoid potential circular imports, NameError, or AttributeError issues.

- Remove _AcceleratorUnverified alias (no longer needed) - Remove duplicate User-Agent flags block from _init_accelerator_config; keep only the early block so the flag is forwarded even when the daemon is blocked - Move _AcceleratorIdentityError handling into _start_accelerator: close daemon, warn 'could not verify daemon identity', return without re-raising - Remove test_binary_path_param_bypasses_lookup (binary_path is a test-only knob already exercised implicitly) - Add test_close_clears_all_state to verify _proc/_tempdir/_uds_path/_log_path/_auth_secret are all None after close() - Add --app-profile to TestCliFlags.test_cli_flags_appended_to_argv - Add assert not t._accelerator_breaker.bypass() to test_non_grpc_exception_reraises in both TestMutateRowViaAccelerator and TestReadRowViaAccelerator Change-Id: Iff4489acc63a6e71211939c23db04ff1a5df10ba

- Remove _AcceleratorUnverified alias (no longer needed) - Remove duplicate User-Agent flags block from _init_accelerator_config; keep only the early block so the flag is forwarded even when the daemon is blocked - Add local import google.cloud.bigtable inside _init_accelerator_config to avoid potential circular import at module level - Move _AcceleratorIdentityError handling into _start_accelerator: close daemon, warn 'could not verify daemon identity', return without re-raising - Remove test_binary_path_param_bypasses_lookup (binary_path is a test-only knob already exercised implicitly) - Add --app-profile to TestCliFlags.test_cli_flags_appended_to_argv - Add assert not t._accelerator_breaker.bypass() to test_non_grpc_exception_reraises in both TestMutateRowViaAccelerator and TestReadRowViaAccelerator Change-Id: I43b354ad6517a2c0b9f6a2294cdd2d1639d7fe8c

- Remove _AcceleratorUnverified alias (no longer needed) - Remove duplicate User-Agent flags block from _init_accelerator_config; keep only the early block so the flag is forwarded even when the daemon is blocked - Move _AcceleratorIdentityError handling into _start_accelerator: close daemon, warn 'could not verify daemon identity', return without re-raising - Remove test_binary_path_param_bypasses_lookup (binary_path is a test-only knob already exercised implicitly) - Add test_close_clears_all_state to verify _proc/_tempdir/_uds_path/_log_path/_auth_secret are all None after close() - Add --app-profile to TestCliFlags.test_cli_flags_appended_to_argv - Add assert not t._accelerator_breaker.bypass() to test_non_grpc_exception_reraises in both TestMutateRowViaAccelerator and TestReadRowViaAccelerator Change-Id: Iff4489acc63a6e71211939c23db04ff1a5df10ba review(bigtable): address PR #18686 reviewer comments - Remove _AcceleratorUnverified alias (no longer needed) - Remove duplicate User-Agent flags block from _init_accelerator_config; keep only the early block so the flag is forwarded even when the daemon is blocked - Add local import google.cloud.bigtable inside _init_accelerator_config to avoid potential circular import at module level - Move _AcceleratorIdentityError handling into _start_accelerator: close daemon, warn 'could not verify daemon identity', return without re-raising - Remove test_binary_path_param_bypasses_lookup (binary_path is a test-only knob already exercised implicitly) - Add --app-profile to TestCliFlags.test_cli_flags_appended_to_argv - Add assert not t._accelerator_breaker.bypass() to test_non_grpc_exception_reraises in both TestMutateRowViaAccelerator and TestReadRowViaAccelerator Change-Id: I43b354ad6517a2c0b9f6a2294cdd2d1639d7fe8c test(bigtable): add TestCleanupTempdir to verify _cleanup_tempdir clears all state Change-Id: I6d6ec51cc039c78d884451502b368862fe8d4ae3

…User-Agent prefixing Pass --caller-user-agent python-v<version> when spawning the daemon so it can construct the full User-Agent prefix (e.g. python-v3.0.0a0-go-accelerator-v3.x.x) for accurate audit logging and traffic attribution. Change-Id: I621217335ad54377a84d44ab4b9af88770dd2958

…patch, and daemon lifecycle

- Remove _AcceleratorUnverified alias (no longer needed) - Remove duplicate User-Agent flags block from _init_accelerator_config; keep only the early block so the flag is forwarded even when the daemon is blocked - Move _AcceleratorIdentityError handling into _start_accelerator: close daemon, warn 'could not verify daemon identity', return without re-raising - Remove test_binary_path_param_bypasses_lookup (binary_path is a test-only knob already exercised implicitly) - Add test_close_clears_all_state to verify _proc/_tempdir/_uds_path/_log_path/_auth_secret are all None after close() - Add --app-profile to TestCliFlags.test_cli_flags_appended_to_argv - Add assert not t._accelerator_breaker.bypass() to test_non_grpc_exception_reraises in both TestMutateRowViaAccelerator and TestReadRowViaAccelerator Change-Id: Iff4489acc63a6e71211939c23db04ff1a5df10ba review(bigtable): address PR #18686 reviewer comments - Remove _AcceleratorUnverified alias (no longer needed) - Remove duplicate User-Agent flags block from _init_accelerator_config; keep only the early block so the flag is forwarded even when the daemon is blocked - Add local import google.cloud.bigtable inside _init_accelerator_config to avoid potential circular import at module level - Move _AcceleratorIdentityError handling into _start_accelerator: close daemon, warn 'could not verify daemon identity', return without re-raising - Remove test_binary_path_param_bypasses_lookup (binary_path is a test-only knob already exercised implicitly) - Add --app-profile to TestCliFlags.test_cli_flags_appended_to_argv - Add assert not t._accelerator_breaker.bypass() to test_non_grpc_exception_reraises in both TestMutateRowViaAccelerator and TestReadRowViaAccelerator Change-Id: I43b354ad6517a2c0b9f6a2294cdd2d1639d7fe8c test(bigtable): add TestCleanupTempdir to verify _cleanup_tempdir clears all state Change-Id: I6d6ec51cc039c78d884451502b368862fe8d4ae3

…e auth session Change-Id: I78f226004fdffe98d83f7d24b04557a24e9fc7a7

…o prevent orphaned daemon on BaseException Change-Id: I6f4c18f5b03da26efa9b70ed8c6b4d20f9d4a8df

sushanb · 2026-10-21T16:59:23Z

                self._accelerator_env["GOOGLE_APPLICATION_CREDENTIALS"] = (
                    credentials_file
                )
-
    def _resolve_principal(self) -> str | None:


Missing blank line between the end of _init_accelerator_config and def _resolve_principal — PEP 8 / black / flake8 (E301) want one. Looks like a CrossSync regen artifact; adding the blank line in the async source and re-running the generator should restore it in the sync file.

Suggested change

self._accelerator_env["GOOGLE_APPLICATION_CREDENTIALS"] = (

credentials_file

)

def _resolve_principal(self) -> str | None:

self._accelerator_env["GOOGLE_APPLICATION_CREDENTIALS"] = (

credentials_file

)

def _resolve_principal(self) -> str | None:

sushanb · 2026-10-21T16:59:36Z

+        import google.cloud.bigtable  # local import avoids circular dependency
+
+        self._accelerator_flags += [
+            "--caller-user-agent",
+            f"python-v{google.cloud.bigtable.__version__}",
+        ]


Pulling in the entire google.cloud.bigtable package on every Table init is overkill when all we want is the version string. Switch to importlib.metadata (stdlib since 3.8), which avoids the circular-dep motivation entirely:

Suggested change

import google.cloud.bigtable # local import avoids circular dependency

self._accelerator_flags += [

"--caller-user-agent",

f"python-v{google.cloud.bigtable.__version__}",

]

from importlib.metadata import version as _pkg_version

self._accelerator_flags += [

"--caller-user-agent",

f"python-v{_pkg_version('google-cloud-bigtable')}",

]

(Can be hoisted to a module-level import if you'd rather; local keeps it self-contained with the use site.)

removed the import line. I think this is already imported.

…n accelerator blocked, regenerate sync client Change-Id: Ia71d293208e7340bf9cab5cfa9f7e81ce7bb20b5

sushanb

LGTM with the inline suggestions above (docstring, user-agent placement, blank-line regen, inf/nan guard, importlib.metadata). None are blockers; happy to see them land as follow-ups or folded in before merge.

mutianf requested a review from a team as a code owner October 13, 2026 19:56

mutianf added this pull request to stack #18689 October 13, 2026 19:56

gemini-code-assist Bot reviewed Oct 13, 2026

View reviewed changes

Comment thread packages/google-cloud-bigtable/google/cloud/bigtable/data/_async/client.py Outdated

mutianf requested a review from sushanb October 13, 2026 19:57

mutianf removed this pull request from stack #18689 October 13, 2026 20:06

mutianf added this pull request to stack #18691 October 13, 2026 20:07

mutianf force-pushed the accel6 branch from d5f5864 to 19aedd0 Compare October 13, 2026 20:34

mutianf changed the title ~~feat(bigtable): pass Python client version to accelerator daemon for User-Agent prefixing~~ feat(bigtable): pass user agent, tests and start timeout env variable Oct 13, 2026

mutianf commented Oct 13, 2026

View reviewed changes

mutianf force-pushed the accel6 branch from bee7932 to 5e1d30d Compare October 14, 2026 01:31

parthea assigned sushanb Oct 16, 2026

mutianf mentioned this pull request Oct 20, 2026

feat(bigtable): raise the UDS channel inbound message cap to 256 MB #18782

Open

Base automatically changed from accel5 to bigtable-accel October 21, 2026 16:52

mutianf added 5 commits October 21, 2026 12:52

test(bigtable): add accelerator unit tests for fallback, routing, dis…

cd69029

…patch, and daemon lifecycle

fix(bigtable): propagate identity error from _start_accelerator; clos…

9e1686b

…e auth session Change-Id: I78f226004fdffe98d83f7d24b04557a24e9fc7a7

fix(bigtable): move _accelerator_daemon assignment inside try block t…

00b8b06

…o prevent orphaned daemon on BaseException Change-Id: I6f4c18f5b03da26efa9b70ed8c6b4d20f9d4a8df

mutianf force-pushed the accel6 branch from 60a2601 to 00b8b06 Compare October 21, 2026 16:52

sushanb reviewed Oct 21, 2026

View reviewed changes

Comment thread packages/google-cloud-bigtable/google/cloud/bigtable/data/_accelerator/_daemon.py Outdated

sushanb reviewed Oct 21, 2026

View reviewed changes

Comment thread packages/google-cloud-bigtable/google/cloud/bigtable/data/_async/client.py Outdated

sushanb reviewed Oct 21, 2026

View reviewed changes

Comment thread packages/google-cloud-bigtable/google/cloud/bigtable/data/_accelerator/_daemon.py Outdated

sushanb reviewed Oct 21, 2026

View reviewed changes

fix(bigtable): reject non-finite startup timeout, skip user-agent whe…

f542b57

…n accelerator blocked, regenerate sync client Change-Id: Ia71d293208e7340bf9cab5cfa9f7e81ce7bb20b5

sushanb approved these changes Oct 21, 2026

View reviewed changes

mutianf merged commit 5f439b9 into bigtable-accel Oct 21, 2026
16 checks passed

mutianf deleted the accel6 branch October 21, 2026 19:40

Navigation Menu

Repository navigation

feat(bigtable): pass user agent, tests and start timeout env variable - #18686

feat(bigtable): pass user agent, tests and start timeout env variable#18686
mutianf merged 6 commits into
bigtable-accelfrom
accel6

mutianf commented Oct 13, 2026 •

edited

Loading

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sushanb Oct 21, 2026

Uh oh!

Uh oh!

sushanb Oct 21, 2026

Uh oh!

mutianf Oct 21, 2026

Uh oh!

sushanb left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

mutianf commented Oct 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sushanb Oct 21, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sushanb Oct 21, 2026

Choose a reason for hiding this comment

Uh oh!

mutianf Oct 21, 2026

Choose a reason for hiding this comment

Uh oh!

sushanb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mutianf commented Oct 13, 2026 •

edited

Loading