From 41424017c19f38129175aabdca8eb5ed63665a5d Mon Sep 17 00:00:00 2001 From: Shachar Langbeheim <98546660+shachlanAmazon@users.noreply.github.com> Date: Tue, 16 Jan 2024 11:58:17 +0200 Subject: [PATCH] Align client connection - no default address. (#790) If the user didn't pass an address, that's a user error. We shouldn't assume that they want to connect to localhost, considering that isn't a common behavior. Co-authored-by: nihohit --- .../configuration/BaseClientConfiguration.java | 3 +-- .../java/glide/managers/ConnectionManager.java | 17 +++-------------- .../glide/managers/ConnectionManagerTest.java | 10 ---------- python/python/glide/config.py | 17 +++++++---------- python/python/glide/redis_client.py | 6 +++--- python/python/tests/test_config.py | 5 ++--- 6 files changed, 16 insertions(+), 42 deletions(-) diff --git a/java/client/src/main/java/glide/api/models/configuration/BaseClientConfiguration.java b/java/client/src/main/java/glide/api/models/configuration/BaseClientConfiguration.java index a15a3ea4b2..7561aa0f3f 100644 --- a/java/client/src/main/java/glide/api/models/configuration/BaseClientConfiguration.java +++ b/java/client/src/main/java/glide/api/models/configuration/BaseClientConfiguration.java @@ -19,8 +19,7 @@ public abstract class BaseClientConfiguration { * list can be partial, as the client will attempt to map out the cluster and find all nodes. If * the server is in standalone mode, only nodes whose addresses were provided will be used by the * client. For example: [ {address:sample-address-0001.use1.cache.amazonaws.com, port:6379}, - * {address: sample-address-0002.use2.cache.amazonaws.com, port:6379} ]. If none are set, a - * default address localhost:6379 will be used. + * {address: sample-address-0002.use2.cache.amazonaws.com, port:6379} ]. */ @Singular private final List addresses; diff --git a/java/client/src/main/java/glide/managers/ConnectionManager.java b/java/client/src/main/java/glide/managers/ConnectionManager.java index af88a0908d..4b21bec78c 100644 --- a/java/client/src/main/java/glide/managers/ConnectionManager.java +++ b/java/client/src/main/java/glide/managers/ConnectionManager.java @@ -1,8 +1,5 @@ package glide.managers; -import static glide.api.models.configuration.NodeAddress.DEFAULT_HOST; -import static glide.api.models.configuration.NodeAddress.DEFAULT_PORT; - import connection_request.ConnectionRequestOuterClass; import connection_request.ConnectionRequestOuterClass.AuthenticationInfo; import connection_request.ConnectionRequestOuterClass.ConnectionRequest; @@ -71,19 +68,11 @@ private ConnectionRequest createConnectionRequest(BaseClientConfiguration config private ConnectionRequest.Builder setupConnectionRequestBuilderBaseConfiguration( BaseClientConfiguration configuration) { ConnectionRequest.Builder connectionRequestBuilder = ConnectionRequest.newBuilder(); - if (!configuration.getAddresses().isEmpty()) { - for (NodeAddress nodeAddress : configuration.getAddresses()) { - connectionRequestBuilder.addAddresses( - ConnectionRequestOuterClass.NodeAddress.newBuilder() - .setHost(nodeAddress.getHost()) - .setPort(nodeAddress.getPort()) - .build()); - } - } else { + for (NodeAddress nodeAddress : configuration.getAddresses()) { connectionRequestBuilder.addAddresses( ConnectionRequestOuterClass.NodeAddress.newBuilder() - .setHost(DEFAULT_HOST) - .setPort(DEFAULT_PORT) + .setHost(nodeAddress.getHost()) + .setPort(nodeAddress.getPort()) .build()); } diff --git a/java/client/src/test/java/glide/managers/ConnectionManagerTest.java b/java/client/src/test/java/glide/managers/ConnectionManagerTest.java index 0eb8f6c074..42419f9703 100644 --- a/java/client/src/test/java/glide/managers/ConnectionManagerTest.java +++ b/java/client/src/test/java/glide/managers/ConnectionManagerTest.java @@ -68,11 +68,6 @@ public void ConnectionRequestProtobufGeneration_DefaultRedisClientConfiguration_ RedisClientConfiguration redisClientConfiguration = RedisClientConfiguration.builder().build(); ConnectionRequest expectedProtobufConnectionRequest = ConnectionRequest.newBuilder() - .addAddresses( - ConnectionRequestOuterClass.NodeAddress.newBuilder() - .setHost(DEFAULT_HOST) - .setPort(DEFAULT_PORT) - .build()) .setTlsMode(ConnectionRequestOuterClass.TlsMode.NoTls) .setClusterModeEnabled(false) .setReadFrom(ConnectionRequestOuterClass.ReadFrom.Primary) @@ -99,11 +94,6 @@ public void ConnectionRequestProtobufGeneration_DefaultRedisClusterClientConfigu RedisClusterClientConfiguration.builder().build(); ConnectionRequest expectedProtobufConnectionRequest = ConnectionRequest.newBuilder() - .addAddresses( - ConnectionRequestOuterClass.NodeAddress.newBuilder() - .setHost(DEFAULT_HOST) - .setPort(DEFAULT_PORT) - .build()) .setTlsMode(ConnectionRequestOuterClass.TlsMode.NoTls) .setClusterModeEnabled(true) .setReadFrom(ConnectionRequestOuterClass.ReadFrom.Primary) diff --git a/python/python/glide/config.py b/python/python/glide/config.py index 3051668fda..40254b8e81 100644 --- a/python/python/glide/config.py +++ b/python/python/glide/config.py @@ -93,7 +93,7 @@ def __init__( class BaseClientConfiguration: def __init__( self, - addresses: Optional[List[NodeAddress]] = None, + addresses: List[NodeAddress], use_tls: bool = False, credentials: Optional[RedisCredentials] = None, read_from: ReadFrom = ReadFrom.PRIMARY, @@ -105,7 +105,7 @@ def __init__( Represents the configuration settings for a Redis client. Args: - addresses (Optional[List[NodeAddress]]): DNS Addresses and ports of known nodes in the cluster. + addresses (List[NodeAddress]): DNS Addresses and ports of known nodes in the cluster. If the server is in cluster mode the list can be partial, as the client will attempt to map out the cluster and find all nodes. If the server is in standalone mode, only nodes whose addresses were provided will be used by the @@ -115,7 +115,6 @@ def __init__( {address:sample-address-0001.use1.cache.amazonaws.com, port:6379}, {address: sample-address-0002.use2.cache.amazonaws.com, port:6379} ]. - If none are set, a default address localhost:6379 will be used. use_tls (bool): True if communication with the cluster should use Transport Level Security. Should match the TLS configuration of the server/cluster, otherwise the connection attempt will fail credentials (RedisCredentials): Credentials for authentication process. @@ -126,7 +125,7 @@ def __init__( If the specified timeout is exceeded for a pending request, it will result in a timeout error. If not set, a default value will be used. client_name (Optional[str]): Client name to be used for the client. Will be used with CLIENT SETNAME command during connection establishment. """ - self.addresses = addresses or [NodeAddress()] + self.addresses = addresses self.use_tls = use_tls self.credentials = credentials self.read_from = read_from @@ -172,14 +171,13 @@ class RedisClientConfiguration(BaseClientConfiguration): Represents the configuration settings for a Standalone Redis client. Args: - addresses (Optional[List[NodeAddress]]): DNS Addresses and ports of known nodes in the cluster. + addresses (List[NodeAddress]): DNS Addresses and ports of known nodes in the cluster. Only nodes whose addresses were provided will be used by the client. For example: [ {address:sample-address-0001.use1.cache.amazonaws.com, port:6379}, {address: sample-address-0002.use2.cache.amazonaws.com, port:6379} ]. - If none are set, a default address localhost:6379 will be used. use_tls (bool): True if communication with the cluster should use Transport Level Security. credentials (RedisCredentials): Credentials for authentication process. If none are set, the client will not authenticate itself with the server. @@ -198,7 +196,7 @@ class RedisClientConfiguration(BaseClientConfiguration): def __init__( self, - addresses: Optional[List[NodeAddress]] = None, + addresses: List[NodeAddress], use_tls: bool = False, credentials: Optional[RedisCredentials] = None, read_from: ReadFrom = ReadFrom.PRIMARY, @@ -244,13 +242,12 @@ class ClusterClientConfiguration(BaseClientConfiguration): Represents the configuration settings for a Cluster Redis client. Args: - addresses (Optional[List[NodeAddress]]): DNS Addresses and ports of known nodes in the cluster. + addresses (List[NodeAddress]): DNS Addresses and ports of known nodes in the cluster. The list can be partial, as the client will attempt to map out the cluster and find all nodes. For example: [ {address:configuration-endpoint.use1.cache.amazonaws.com, port:6379} ]. - If none are set, a default address localhost:6379 will be used. use_tls (bool): True if communication with the cluster should use Transport Level Security. credentials (RedisCredentials): Credentials for authentication process. If none are set, the client will not authenticate itself with the server. @@ -268,7 +265,7 @@ class ClusterClientConfiguration(BaseClientConfiguration): def __init__( self, - addresses: Optional[List[NodeAddress]] = None, + addresses: List[NodeAddress], use_tls: bool = False, credentials: Optional[RedisCredentials] = None, read_from: ReadFrom = ReadFrom.PRIMARY, diff --git a/python/python/glide/redis_client.py b/python/python/glide/redis_client.py index da7ae8e42a..13eb20a8f7 100644 --- a/python/python/glide/redis_client.py +++ b/python/python/glide/redis_client.py @@ -60,17 +60,17 @@ def __init__(self, config: BaseClientConfiguration): self._is_closed: bool = False @classmethod - async def create(cls, config: Optional[BaseClientConfiguration] = None) -> Self: + async def create(cls, config: BaseClientConfiguration) -> Self: """Creates a Redis client. Args: - config (Optional[ClientConfiguration]): The client configurations. + config (ClientConfiguration): The client configurations. If no configuration is provided, a default client to "localhost":6379 will be created. Returns: Self: a Redis Client instance. """ - config = config or BaseClientConfiguration() + config = config self = cls(config) init_future: asyncio.Future = asyncio.Future() loop = asyncio.get_event_loop() diff --git a/python/python/tests/test_config.py b/python/python/tests/test_config.py index 4f502586e6..17fce5403c 100644 --- a/python/python/tests/test_config.py +++ b/python/python/tests/test_config.py @@ -5,9 +5,8 @@ def test_default_client_config(): - config = BaseClientConfiguration() - assert config.addresses[0].host == "localhost" - assert config.addresses[0].port == 6379 + config = BaseClientConfiguration([]) + assert len(config.addresses) == 0 assert config.read_from.value == ProtobufReadFrom.Primary assert config.use_tls is False assert config.client_name is None