fix(bigtable): correct get_bigtable_data_client return annotation#6313
Open
anxkhn wants to merge 1 commit into
Open
fix(bigtable): correct get_bigtable_data_client return annotation#6313anxkhn wants to merge 1 commit into
anxkhn wants to merge 1 commit into
Conversation
The return annotation referenced bigtable.BigtableDataClient, but that name is not exported from the google.cloud.bigtable top-level namespace (only Client is). The client is constructed from google.cloud.bigtable.data, which is where BigtableDataClient actually lives and is already imported. Because of "from __future__ import annotations" the annotation is a lazy string, so it does not fail at runtime, but typing.get_type_hints() and doc tooling raise AttributeError when resolving it. Point the annotation at data.BigtableDataClient (the type actually returned) and add a regression test that get_type_hints() resolves it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Please ensure you have read the contribution guide before creating a pull request.
Link to Issue or Description of Change
1. Link to an existing issue (if applicable):
N/A (no existing issue; described below per CONTRIBUTING for a small fix).
2. Or, if no issue exists, describe the change:
Problem:
In
src/google/adk/tools/bigtable/client.py,get_bigtable_data_clientisannotated as returning
bigtable.BigtableDataClient:BigtableDataClientis not exported from thegoogle.cloud.bigtabletop-levelnamespace; that package only exports
Client:The type actually constructed and returned is
google.cloud.bigtable.data.BigtableDataClient,and
datais already imported in the module. Because the module usesfrom __future__ import annotations, the annotation is a lazy string, so thisdoes not raise at import time. But it is a wrong type reference: any tooling that
resolves the annotation fails. For example:
This breaks
typing.get_type_hints(), and by extension type checkers and API-docgenerators that resolve return annotations. The sibling
get_bigtable_admin_clientin the same file is correctly annotated
-> bigtable.Client.Solution:
Point the annotation at the type that is actually constructed and returned,
data.BigtableDataClient(import already present). One-character-scope change; nobehavior change. The
from google.cloud import bigtableimport is kept becauseget_bigtable_admin_clientstill usesbigtable.Client.Testing Plan
Unit Tests:
Added
test_get_bigtable_data_client_return_annotation_resolvesintests/unittests/tools/bigtable/test_client.py, asserting the return annotationnow resolves to the real type:
The test is a genuine regression guard: with the old annotation it fails with
AttributeError: module 'google.cloud.bigtable' has no attribute 'BigtableDataClient';with the fix it passes.
pytestresults:Manual End-to-End (E2E) Tests:
Not applicable: this is a type-annotation-only correction with no runtime behavior
change (the returned object is unchanged). Resolution of the annotation is
demonstrated above and covered by the new unit test.
Checklist
Additional context
Precedent for this class of type-hint fix being accepted in this repo: maintainer
commit
9865e2bc("fix(live): fix the return type hint of handle_function_calls_live"),plus
1ce043a2andf592de4c.