Skip to content

Commit

Permalink
Align client connection - no default address. (#790)
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
shachlanAmazon and nihohit authored Jan 16, 2024
1 parent 7f50ec1 commit 4142401
Show file tree
Hide file tree
Showing 6 changed files with 16 additions and 42 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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: <code>[ {address:sample-address-0001.use1.cache.amazonaws.com, port:6379},
* {address: sample-address-0002.use2.cache.amazonaws.com, port:6379} ]</code>. If none are set, a
* default address localhost:6379 will be used.
* {address: sample-address-0002.use2.cache.amazonaws.com, port:6379} ]</code>.
*/
@Singular private final List<NodeAddress> addresses;

Expand Down
17 changes: 3 additions & 14 deletions java/client/src/main/java/glide/managers/ConnectionManager.java
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down
17 changes: 7 additions & 10 deletions python/python/glide/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand All @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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,
Expand Down Expand Up @@ -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.
Expand All @@ -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,
Expand Down
6 changes: 3 additions & 3 deletions python/python/glide/redis_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
5 changes: 2 additions & 3 deletions python/python/tests/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 4142401

Please sign in to comment.