Repository navigation
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request improves scope normalization in both the async and sync Bigtable clients by correctly handling lists of scopes that contain comma-separated values and leading/trailing whitespace. It also updates the accelerator configuration to use these normalized, sorted scopes and adds comprehensive unit tests to verify the new behavior. The review feedback suggests simplifying the generator expressions in _normalize_scopes by replacing commas with spaces and using split() without arguments, which would automatically strip whitespace and filter out empty strings.
| return frozenset( | ||
| part.strip() | ||
| for s in scopes | ||
| if isinstance(s, str) | ||
| for part in s.split(",") | ||
| if part.strip() | ||
| ) |
There was a problem hiding this comment.
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()
)| return frozenset( | ||
| ( | ||
| part.strip() | ||
| for s in scopes | ||
| if isinstance(s, str) | ||
| for part in s.split(",") | ||
| if part.strip() | ||
| ) | ||
| ) |
There was a problem hiding this comment.
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()
)
…alize_scopes
Fixes b/578435999 🦕