chore(spanner): deprecate experimental host option/parameter to replace with Spanner Omni#17246
chore(spanner): deprecate experimental host option/parameter to replace with Spanner Omni#17246sagnghos wants to merge 3 commits into
Conversation
…ce with Spanner Omni
There was a problem hiding this comment.
Code Review
This pull request deprecates the experimental_host parameter across the Spanner client library, replacing it with a new instance_type parameter (supporting "cloud", "omni", and "emulator") and transitioning Spanner Omni connections to use standard client_options with api_endpoint. Feedback on this PR highlights critical issues regarding dictionary handling for client_options in both sync and async client constructors, potential AttributeErrors in Database classes if self._instance is None, and the unintended discarding of custom client options in the DB-API connection module.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request deprecates the experimental_host parameter in favor of using client_options with api_endpoint and setting instance_type="omni" to connect to Spanner Omni instances. It introduces an InstanceType class containing constants for different instance types and updates transport creation helpers, client initialization, database session managers, and tests to support this new configuration. The review comments suggest two improvements: setting self._instance_type to InstanceType.EMULATOR when the emulator host is active in both sync and async clients, and using getattr defensively to retrieve _client from self._database._instance in both sync and async database session managers.
This PR deprecates the
experimental_hostparameter across publicClient,AsyncClient, and DB-APIconnectinterfaces, introducing the new unifiedinstance_typeparameter.The
instance_typeoption supportscloudandomniconnection types; setting the type tocloudcurrently acts as a no-op, whereas establishing a connection to a Spanner Omni instance strictly requires settinginstance_type="omni".Internally, all private
_experimental_hostattributes have been purged and refactored to dynamically resolve endpoints via client properties, all public docs-strings have been updated to describe this behavior.Refer to discussion: Spanner Client Library Configuration for Omni
Similar PR in java-spanner googleapis/google-cloud-java#13236