feat: support row_range in sample_row_keys method#17330
Conversation
- In Table.sample_row_keys, updated the docstring to clarify that the end key of the provided row range is always returned as the last sample (or empty string if no range is specified), and updated the offset_bytes description to specify it includes preceding rows but after the last sample before the range. - In async and sync unit tests, updated the mocked SampleRowKeys stream responses to use realistic keys falling inside the RowRange bounds (e.g. b"a_key1" and b"b"), and updated the key/length assertions accordingly. - Reverted manual GAPIC type patches to let OwlBot/Librarian auto-patching handle the API class fields natively upon proto submission. TAG=agy CONV=6cea44b5-6adb-45c6-b8c8-58b37f95584c
![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 adds support for a row_range parameter in the sample_row_keys method across the asynchronous, synchronous autogenerated, and legacy Bigtable clients, allowing users to restrict sampled row keys to a specific range. It also updates the test proxy handlers and adds corresponding unit tests. The review feedback highlights a potential bug in client_handler_legacy.py where an empty byte string b"" could be incorrectly treated as falsy when resolving range keys. Additionally, it suggests encapsulating None checks within helper functions and using explicit is not None checks instead of truthiness checks for optional parameters.
| row_range = None | ||
| if "row_range" in request: | ||
| from google.cloud.bigtable.data.read_rows_query import RowRange | ||
| row_range = RowRange._from_dict(request["row_range"]) |
There was a problem hiding this comment.
When processing optional parameters with a helper function, prefer placing the null or None check within the function body rather than at the call site to simplify the calling code and improve encapsulation. Instead of checking if row_range is in the request at the call site, pass the optional value directly to RowRange._from_dict and handle the None check inside that helper function.
from google.cloud.bigtable.data.read_rows_query import RowRange
row_range = RowRange._from_dict(request.get("row_range"))References
- When processing optional parameters with a helper function, prefer placing the null or None check within the function body rather than at the call site to simplify the calling code and improve encapsulation.
| row_range = None | ||
| if "row_range" in request: | ||
| from google.cloud.bigtable.data.read_rows_query import RowRange | ||
|
|
||
| row_range = RowRange._from_dict(request["row_range"]) |
There was a problem hiding this comment.
When processing optional parameters with a helper function, prefer placing the null or None check within the function body rather than at the call site to simplify the calling code and improve encapsulation. Instead of checking if row_range is in the request at the call site, pass the optional value directly to RowRange._from_dict and handle the None check inside that helper function.
from google.cloud.bigtable.data.read_rows_query import RowRange
row_range = RowRange._from_dict(request.get("row_range"))References
- When processing optional parameters with a helper function, prefer placing the null or None check within the function body rather than at the call site to simplify the calling code and improve encapsulation.
…ler_legacy.py Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
anvitTawar
changed the title
…nc/client.py Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
…c_autogen/client.py Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
2 participants
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #17329