-
Notifications
You must be signed in to change notification settings - Fork 62
chore: Moved admin client to google.cloud.bigtable.admin/admin_v2 #1204
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.
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
base: v3_staging
Are you sure you want to change the base?
Conversation
|
||
data_client/data_client_usage | ||
classic_client/usage | ||
admin_client/admin_client_usage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I probably would have preferred to have the new docs as a separate PR to keep things focused, but probably not a big deal
@@ -0,0 +1,274 @@ | |||
# -*- coding: utf-8 -*- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we have to have both google.cloud.bigtable.admin
and google.cloud.bigtable.admin_v2
? I thought we'd just need google.cloud.bigtable.admin
It seems confusing to have google.cloud.bigtable.admin
, google.cloud.bigtable.admin_v2
, google.cloud.bigtable_admin
, and google.cloud.bigtable.admin_v2
. Maybe we should reconsider
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The GAPIC structure is that google.cloud.bigtable.admin
aliases to google.cloud.bigtable.admin_v<version>
# | ||
from .client import BigtableTableAdminClient | ||
|
||
# TODO: Add the async client after owlbot changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this mean?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Old TODO comment from before that I should delete
|
||
# -------------------------------------------------------------------------- | ||
# Admin Overlay work | ||
# -------------------------------------------------------------------------- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit confused why these new owlbot changes have to be added? Why are they only needed after this move?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
v3_staging
doesn't have the selective GAPIC client in it right now, so I added it back in.
@@ -0,0 +1,57 @@ | |||
# -*- coding: utf-8 -*- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need to merge main into v3_staging
?
Maybe we should make a small doc to make sure we're all aligned on what the final structure will look like? I'd hate to spend a lot of time iterating here if we have to move things back and forth a bunch Good to see import aliases aren't going to be a problem though |
Changes:
google.cloud.bigtable_admin/bigtable_admin_v2
togoogle.cloud.bigtable.admin/admin_v2
. This is more in line with what was initially planned. The autogenerated portion of the library was generated as follows:This change will be committed close to when we will release the v3 client.
bigtable_admin/bigtable_admin_v2
still work as intended.BigtableTableAdminClient
instead of theBaseBigtableTableAdminClient
as its table admin client.