Skip to content

feat(bigtable): update legacy escape hatch to return V2 admin clients#13375

Open
jinseopkim0 wants to merge 2 commits into
mainfrom
bigtable-escape-hatch
Open

feat(bigtable): update legacy escape hatch to return V2 admin clients#13375
jinseopkim0 wants to merge 2 commits into
mainfrom
bigtable-escape-hatch

Conversation

@jinseopkim0
Copy link
Copy Markdown
Contributor

@jinseopkim0 jinseopkim0 commented Jun 5, 2026

This PR updates the legacy BigtableTableAdminClient and BigtableInstanceAdminClient escape hatches (getBaseClient()) to return the modern V2 admin clients (BigtableTableAdminClientV2 and BigtableInstanceAdminClientV2) instead of the autogenerated base clients (BaseBigtableTableAdminClient and BaseBigtableInstanceAdminClient).

@jinseopkim0 jinseopkim0 requested review from a team as code owners June 5, 2026 16:03
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request transitions BigtableInstanceAdminClient and BigtableTableAdminClient to use the V2 client classes (BigtableInstanceAdminClientV2 and BigtableTableAdminClientV2) instead of the base autogenerated clients. Key feedback includes resolving a compilation error in BigtableTableAdminClientV2.createAwaitConsistencyCallable due to a type mismatch with EnhancedBigtableTableAdminStub, and updating outdated Javadocs for getBaseClient() in both admin clients to accurately reflect the new V2 return types.

@jinseopkim0 jinseopkim0 marked this pull request as draft June 5, 2026 16:07
@jinseopkim0 jinseopkim0 force-pushed the bigtable-escape-hatch branch 2 times, most recently from 286f727 to 5ac8d52 Compare June 5, 2026 16:14
@jinseopkim0
Copy link
Copy Markdown
Contributor Author

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors BigtableInstanceAdminClient and BigtableTableAdminClient to use the newer V2 client implementations instead of the older Base clients. The review comments correctly identify compilation errors in BigtableTableAdminClient where EnhancedBigtableTableAdminStub is passed to methods expecting GrpcBigtableTableAdminStub. To fix this, the reviewer suggests exposing the underlying GrpcBigtableTableAdminStub from EnhancedBigtableTableAdminStub and using it during the client initialization.

@jinseopkim0 jinseopkim0 force-pushed the bigtable-escape-hatch branch from 5ac8d52 to 802f22e Compare June 5, 2026 16:45
@jinseopkim0 jinseopkim0 force-pushed the bigtable-escape-hatch branch from 802f22e to e69b16c Compare June 5, 2026 16:58
@jinseopkim0 jinseopkim0 marked this pull request as ready for review June 5, 2026 17:08
private final String projectId;
private final BigtableInstanceAdminStub stub;
private final BaseBigtableInstanceAdminClient baseClient;
private final BigtableInstanceAdminClientV2 baseClient;
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.

nit, can we rename this to V2Client?

boolean shouldAutoClose =
stub.getSettings().getBackgroundExecutorProvider().shouldAutoClose();

AwaitConsistencyCallableV2 awaitConsistencyCallable =
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.

wait this is a bit weird, why do we need to create the callable seperately instead of letting the V2 client create the callable?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants