Repository navigation
chore(bigtable): remove legacy session/channel config migration in ClientConfigurationManager - #14891
Conversation
…ientConfigurationManager
![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 removes legacy configuration migration logic for SessionPoolConfiguration and ChannelPoolConfiguration from ClientConfigurationManager, along with its associated tests. Additionally, in SessionDeadlineTest, the mock getClientConfiguration method is updated to load the default configuration dynamically. Feedback on this change suggests caching the default configuration to avoid inefficient classpath I/O and protobuf parsing on every gRPC request, and wrapping the IOException in a StatusRuntimeException for idiomatic gRPC error propagation.
| ClientConfiguration.Builder builder; | ||
| try { | ||
| builder = ClientConfigurationManager.loadDefault().toBuilder(); | ||
| } catch (IOException e) { | ||
| responseObserver.onError(e); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Loading and parsing the default configuration on every gRPC request in the mock service is inefficient because ClientConfigurationManager.loadDefault() performs classpath resource I/O and protobuf parsing. To optimize this, consider loading the default configuration once and caching it in a private static final field within the class. Additionally, propagating a raw IOException directly to responseObserver.onError is less idiomatic in gRPC; wrapping it in a StatusRuntimeException (e.g., Status.INTERNAL.withCause(e).asException()) is preferred.
Removes the legacy
SessionPoolConfiguration(LoadBalancingOptions) andChannelPoolConfigurationmigration blocks fromClientConfigurationManager.normalizeConfignow that the server populates them directly.