Repository navigation
docs(bigtable): add samples for sync client - #17820
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 adds new Python samples and tests for the Google Cloud Bigtable admin and data clients, while updating legacy client samples with _legacy region tags. The review feedback highlights several critical API usage issues in the new data client snippets, including incorrect attribute access on Row and Cell objects, the use of the non-existent DeleteRangeFromColumn class (which should be DeleteCells), and type mismatches where regex filters expect bytes instead of strings (or vice-versa). Additionally, the feedback suggests minor cleanups for unused imports, unused variables, and redundant fully qualified imports.
daniel-sanche
changed the title
mutianf
left a comment
mutianf
left a comment
There was a problem hiding this comment.
Do we need to keep the legacy/ samples? or we can remove them?
| # [START bigtable_check_instance_exists] | ||
| from google.api_core.exceptions import NotFound | ||
|
|
||
| from google.cloud import bigtable_admin |
There was a problem hiding this comment.
should we import from bigtable_admin_v2 to be clear that this is using the gapic generated client?
There was a problem hiding this comment.
bigtable_admin is the version-less alias for bigtable_admin_v2, which is what we usually point to in python sdk samples.
But I agree the clients are kind of confusing here, so if you think having _v2 makes things clearer, that works for me. Let me know
| @@ -0,0 +1,2 @@ | |||
| google-cloud-bigtable | |||
There was a problem hiding this comment.
do we need a minimum version? (after gapic generated client is released?)
There was a problem hiding this comment.
We shouldn't need one, because the expectation is that bigtable is installed at HEAD. But I'll make the change in case anyone wants to run against the libraries manually
| print( | ||
| "Command should be either run or delete.\n Use argument -h,\ | ||
| --help to show help and exit." | ||
| ) |
There was a problem hiding this comment.
should we add example for restore_table and wait_for_consistency (since those are overlays)?
There was a problem hiding this comment.
Good idea, added
Although the other samples are being imported into devsite samples, and these only live here in the repo for now
I was thinking we could keep them here in the repo, in case we need to link to them later. But the plan is to remove them from the public docs |
fb1b2bc to
d62d731
Compare
Add samples for the clients that replace the deprecated `google.cloud.bigtable.Client`: - samples/data_client: synchronous data samples (hello, quickstart, reads/writes/filters/deletes snippets) using BigtableDataClient - samples/admin_client: instance, table and metric-scaler admin samples using the generated BigtableInstanceAdminClient / BigtableTableAdminClient, including the overlay-only wait_for_consistency, create_backup and restore_table operations The new sync samples take over the canonical region tags; the legacy client samples are renamed to `*_legacy` and kept as tested references. The async samples drop their dependency on the legacy client for table administration, and the shared test utilities provision tables through the admin client.
d62d731 to
2746ed3
Compare
Some samples are still implemented in the legacy client only. This PR implements them in the data or admin clients, and adds a suffix to the region tags for the legacy client
Some samples are still implemented in the legacy client only. This PR implements them in the data or admin clients, and adds a suffix to the region tags for the legacy client