Skip to content

fix(bigtable): normalize scopes in _init_accelerator_config and _norm… #19031

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mutianf wants to merge 1 commit into
base: bigtable-accel
Choose a base branch
from
Open

fix(bigtable): normalize scopes in _init_accelerator_config and _norm… #19031

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 15 additions & 4 deletions packages/google-cloud-bigtable/google/cloud/bigtable/data/_async/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,10 +236,16 @@ def _normalize_scopes(scopes: Any) -> frozenset[str]:
if not scopes:
return frozenset()
if isinstance(scopes, str):
scopes = scopes.replace(",", " ").split()
elif not isinstance(scopes, (list, tuple, set, frozenset)):
return frozenset(scopes.replace(",", " ").split())
if not isinstance(scopes, (list, tuple, set, frozenset)):
return frozenset()
return frozenset(s.strip() for s in scopes if isinstance(s, str) and s.strip())
return frozenset(
part.strip()
for s in scopes
if isinstance(s, str)
for part in s.split(",")
if part.strip()
)
Comment on lines +242 to +248

@gemini-code-assist gemini-code-assist Bot Nov 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

We can simplify the generator expression by replacing commas with spaces and using split() without arguments. This aligns the list-element parsing behavior with the string parsing behavior (which also splits on whitespace and commas), and leverages Python's optimized split() behavior to automatically strip whitespace and filter out empty strings, making the code cleaner and more robust.

    return frozenset(
        part
        for s in scopes
        if isinstance(s, str)
        for part in s.replace(",", " ").split()
    )



def _credential_type(creds: Any) -> str | None:
Expand Down Expand Up @@ -498,7 +504,12 @@ def _init_accelerator_config(
# Forward the effective auth scopes so the daemon's token audience
# matches the native client (whose default is TransportType.AUTH_SCOPES),
# rather than the daemon's single hardcoded data scope.
effective_scopes = list(scopes) if scopes else list(TransportType.AUTH_SCOPES)
normalized_scopes = _normalize_scopes(scopes)
effective_scopes = (
sorted(normalized_scopes)
if normalized_scopes
else list(TransportType.AUTH_SCOPES)
)
# Remember the scopes we forwarded so the identity handshake can confirm
# the daemon actually resolved a token for the same effective scope.
self._accelerator_scopes: list[str] = effective_scopes
Expand Down
21 changes: 17 additions & 4 deletions packages/google-cloud-bigtable/google/cloud/bigtable/data/_sync_autogen/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,10 +174,18 @@ def _normalize_scopes(scopes: Any) -> frozenset[str]:
if not scopes:
return frozenset()
if isinstance(scopes, str):
scopes = scopes.replace(",", " ").split()
elif not isinstance(scopes, (list, tuple, set, frozenset)):
return frozenset(scopes.replace(",", " ").split())
if not isinstance(scopes, (list, tuple, set, frozenset)):
return frozenset()
return frozenset((s.strip() for s in scopes if isinstance(s, str) and s.strip()))
return frozenset(
(
part.strip()
for s in scopes
if isinstance(s, str)
for part in s.split(",")
if part.strip()
)
)
Comment on lines +180 to +188

@gemini-code-assist gemini-code-assist Bot Nov 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

We can simplify the generator expression by replacing commas with spaces and using split() without arguments. This aligns the list-element parsing behavior with the string parsing behavior, leverages Python's optimized split() behavior to automatically strip whitespace and filter out empty strings, and removes the redundant extra parentheses around the generator expression.

    return frozenset(
        part
        for s in scopes
        if isinstance(s, str)
        for part in s.replace(",", " ").split()
    )



def _credential_type(creds: Any) -> str | None:
Expand Down Expand Up @@ -379,7 +387,12 @@ def _init_accelerator_config(
f"python-v{google.cloud.bigtable.__version__}",
]
scopes = getattr(client_options, "scopes", None) if client_options else None
effective_scopes = list(scopes) if scopes else list(TransportType.AUTH_SCOPES)
normalized_scopes = _normalize_scopes(scopes)
effective_scopes = (
sorted(normalized_scopes)
if normalized_scopes
else list(TransportType.AUTH_SCOPES)
)
self._accelerator_scopes: list[str] = effective_scopes
if effective_scopes:
self._accelerator_flags += ["--scopes", ",".join(effective_scopes)]
Expand Down
120 changes: 99 additions & 21 deletions packages/google-cloud-bigtable/tests/unit/data/test_accelerator_credentials.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,28 @@ def __init__(self, email=None):
self.service_account_email = email


_CLIENT_MODS = [
pytest.param(
(
_async_client_mod.BigtableDataClientAsync,
_async_client_mod._DataApiTargetAsync,
_async_client_mod._AcceleratorIdentityError,
_async_client_mod._UNSET,
),
id="async",
),
pytest.param(
(
_sync_client_mod.BigtableDataClient,
_sync_client_mod._DataApiTarget,
_sync_client_mod._AcceleratorIdentityError,
_sync_client_mod._UNSET,
),
id="sync",
),
]


class TestInitAcceleratorConfig:
def test_no_config_forwards_default_scopes(self):
c = _bare_client()
Expand Down Expand Up @@ -148,6 +170,73 @@ def test_caller_user_agent_not_built_when_blocked(self):
assert c._accelerator_blocked_reason is not None
assert c._accelerator_flags == []

@pytest.mark.parametrize("client_mod", _CLIENT_MODS)
def test_scopes_list_whitespace_trimmed_and_blank_dropped(self, client_mod):
client_cls, _, _, unset = client_mod
c = _bare_client(client_cls, unset)
c._init_accelerator_config(
explicit_credentials=False,
client_options=ClientOptions(
scopes=[
" https://www.googleapis.com/auth/bigtable.data ",
" https://www.googleapis.com/auth/cloud-platform",
" ",
"",
]
),
)
expected = [
"https://www.googleapis.com/auth/bigtable.data",
"https://www.googleapis.com/auth/cloud-platform",
]
assert c._accelerator_scopes == expected
flags = c._accelerator_flags
assert flags[flags.index("--scopes") + 1] == ",".join(expected)

@pytest.mark.parametrize("client_mod", _CLIENT_MODS)
def test_scopes_string_not_exploded_into_characters(self, client_mod):
client_cls, _, _, unset = client_mod
single = "https://www.googleapis.com/auth/bigtable.data"
c1 = _bare_client(client_cls, unset)
c1._init_accelerator_config(
explicit_credentials=False,
client_options=ClientOptions(scopes=single),
)
assert c1._accelerator_scopes == [single]
flags1 = c1._accelerator_flags
assert flags1[flags1.index("--scopes") + 1] == single

multi = (
"https://www.googleapis.com/auth/bigtable.data, "
"https://www.googleapis.com/auth/cloud-platform"
)
c2 = _bare_client(client_cls, unset)
c2._init_accelerator_config(
explicit_credentials=False,
client_options=ClientOptions(scopes=multi),
)
expected = [
"https://www.googleapis.com/auth/bigtable.data",
"https://www.googleapis.com/auth/cloud-platform",
]
assert c2._accelerator_scopes == expected
flags2 = c2._accelerator_flags
assert flags2[flags2.index("--scopes") + 1] == ",".join(expected)

@pytest.mark.parametrize("client_mod", _CLIENT_MODS)
def test_scopes_blank_falls_back_to_default_auth_scopes(self, client_mod):
client_cls, _, _, unset = client_mod
default_scopes = list(_async_client_mod.TransportType.AUTH_SCOPES)
for blank in ([" ", ""], " , "):
c = _bare_client(client_cls, unset)
c._init_accelerator_config(
explicit_credentials=False,
client_options=ClientOptions(scopes=blank),
)
assert c._accelerator_scopes == default_scopes
flags = c._accelerator_flags
assert flags[flags.index("--scopes") + 1] == ",".join(default_scopes)


class TestResolvePrincipal:
def test_service_account_email(self):
Expand Down Expand Up @@ -193,27 +282,6 @@ def fake_refresh(request):
_SCOPES = ["https://www.googleapis.com/auth/bigtable.data"]
_CRED_TYPE = "service_account"

_CLIENT_MODS = [
pytest.param(
(
_async_client_mod.BigtableDataClientAsync,
_async_client_mod._DataApiTargetAsync,
_async_client_mod._AcceleratorIdentityError,
_async_client_mod._UNSET,
),
id="async",
),
pytest.param(
(
_sync_client_mod.BigtableDataClient,
_sync_client_mod._DataApiTarget,
_sync_client_mod._AcceleratorIdentityError,
_sync_client_mod._UNSET,
),
id="sync",
),
]


@pytest.mark.parametrize("client_mod", _CLIENT_MODS)
class TestResolveCredentialType:
Expand Down Expand Up @@ -423,6 +491,16 @@ def test_scope_match_ignores_order_and_dupes(self, client_mod):
target_cls=target_cls,
)

def test_scope_list_whitespace_and_comma_elements_match(self, client_mod):
_, target_cls, _, _ = client_mod
_verify(
"svc@proj.iam.gserviceaccount.com",
"svc@proj.iam.gserviceaccount.com",
client_scopes=[" scope-a ", "scope-b, scope-c "],
daemon_scopes=["scope-a", "scope-b", "scope-c"],
target_cls=target_cls,
)

def test_scope_string_and_non_iterable_handled(self, client_mod):
_, target_cls, err_cls, _ = client_mod
_verify(
Expand Down
Loading