valkey-io / valkey-py

Valkey Python client based on a fork of redis-py
MIT License
69 stars 9 forks source link

ConnectionPool connection error #110

Closed amirreza8002 closed 1 month ago

amirreza8002 commented 1 month ago

hi something i can't find is, when i pass in connection pool to valkey.Valkey, if the server is not available, connect() won't raise an exception, and exception will be raised when i call a method like get(key).

but without a connection pool, connect() will raise an error.

amirreza8002 commented 1 month ago

on a new note, the class valkey.cluster.NodesManager has an options for connection pools even the variable VALKEY_ALLOWED_KEYS includes connection pools

but ValkeyCluster and the base class don't have the option to include

now i don't insist in adding one, since there could be a reason why it's not there, but i'd like to know a workaround for the connection error problem, if there is any

aiven-sal commented 1 month ago

Hi, I'm sorry but I'm not sure I understand what is the problem

amirreza8002 commented 1 month ago

hi

i want to understand how the exception is raised in cluster and if i can control it. my guess is that it's because cluster doesn't support connection pool

in other clients, we have an option to completely ignore connection errors, but moving to cluster those codes don't work anymore.

aiven-sal commented 1 month ago

What's happening here is that the non-cluster connection is performed lazily the first time that it is needed, while cluster performs the connections immediately, cause it needs to reach at least one startup node to be able to setup a cluster.

I don't think there is much that can be done about this. If the connection to the server doesn't work, the client doesn't work anyway. This isn't an error that you can ignore and expect valkey-py to work.

You could lazily create the cluster object when you need it, if that is the kind of behavior that you are looking for.

amirreza8002 commented 1 month ago

hi thank you for the time you put on this

I'll probably just document this feature as not available for cluster.

that is, if i can find why cluster CI is breaking :)

anyway, thanks again 🙌