Repository: redis/redis-py
Pull request: Remove verbose logging from `redis-py/redis/cluster.py`
### Pull Request check-list

_Please make sure to review and check all of these items:_

- [x] Does `$ tox` pass with this change (including linting)?
- [x] Do the CI tests pass with this change (enable it first in your forked repo and wait for the github action build to finish)?
- [ ] Is the new or changed code fully tested?
- [ ] Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?
- [ ] Is there an example added to the examples folder (if applicable)?
- [x] Was the change added to CHANGES file?

_NOTE: these things are not required to open a PR and can be done
afterwards / while the PR is open._

### Description of change
- This PR aims to resolve this [question](https://github.com/redis/redis-py/issues/2227#issuecomment-1159198938) around removing the logging present in `redis-py/redis/cluster.py`.
- Created to close #2227 


Problem statement:
[Question] Remove Redis Cluster logs 
**Version**: 
4.2.2

**Platform**:
Python 3.8.10 on Ubuntu 20.04

**Description**:
There is so much logs in `cluster` module that I would not want it to show up in my application logs. Is there a reason to leave these logs in this module? Instead, couldn't we just raise exception and let the user handle them as he wishes?

Hints from issue discussion:
Agreed. Those shouldn't have made it through the PR. However, I'd argue that this is a breaking (but worthwhile!) change, in case someone is relying on this behaviour.  
We made a similar change in the latest Jedis.

@dvora-h candidate for >=4.4.0
In theory, are we saying here that we would like to remove all of the `log.` statements from *redis-py/redis/cluster.py*?
I think that would be best. Log statements probably get ignored & add significant overhead even if it is not within debugging level (the LogRecord objects themselves take a lot of time to create & then are instantly thrown away).
This is the approach I've followed for the async version too.

Commit message: updated CHANGES

Patch to review:
diff --git a/CHANGES b/CHANGES
index b7e238ebb3..e461806db6 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,4 +1,4 @@
-
+    * Remove verbose logging from cluster.py
     * Allow negative `retries` for `Retry` class to retry forever
     * Add `items` parameter to `hset` signature
     * Create codeql-analysis.yml (#1988). Thanks @chayim
diff --git a/redis/cluster.py b/redis/cluster.py
index 8e4c654c6b..7f245aad66 100644
--- a/redis/cluster.py
+++ b/redis/cluster.py
@@ -1,5 +1,4 @@
 import copy
-import logging
 import random
 import socket
 import sys
@@ -38,8 +37,6 @@
     str_if_bytes,
 )
 
-log = logging.getLogger(__name__)
-
 
 def get_node_name(host: str, port: int) -> str:
     return f"{host}:{port}"
@@ -571,7 +568,6 @@ def __init__(
                 " RedisCluster(startup_nodes=[ClusterNode('localhost', 6379),"
                 " ClusterNode('localhost', 6378)])"
             )
-        log.debug(f"startup_nodes : {startup_nodes}")
         # Update the connection arguments
         # Whenever a new connection is established, RedisCluster's on_connect
         # method should be run
@@ -701,13 +697,8 @@ def set_default_node(self, node):
         :return True if the default node was set, else False
         """
         if node is None or self.get_node(node_name=node.name) is None:
-            log.info(
-                "The requested node does not exist in the cluster, so "
-                "the default node was not changed."
-            )
             return False
         self.nodes_manager.default_node = node
-        log.info(f"Changed the default cluster node to {node}")
         return True
 
     def monitor(self, target_node=None):
@@ -851,8 +842,6 @@ def _determine_nodes(self, *args, **kwargs):
         else:
             # get the nodes group for this command if it was predefined
             command_flag = self.command_flags.get(command)
-        if command_flag:
-            log.debug(f"Target node/s for {command}: {command_flag}")
         if command_flag == self.__class__.RANDOM:
             # return a random node
             return [self.get_random_node()]
@@ -876,7 +865,6 @@ def _determine_nodes(self, *args, **kwargs):
             node = self.nodes_manager.get_node_from_slot(
                 slot, self.read_from_replicas and command in READ_COMMANDS
             )
-            log.debug(f"Target for {args}: slot {slot}")
             return [node]
 
     def _should_reinitialized(self):
@@ -1094,10 +1082,6 @@ def _execute_command(self, target_node, *args, **kwargs):
                     )
                     moved = False
 
-                log.debug(
-                    f"Executing command {command} on target node: "
-                    f"{target_node.server_type} {target_node.name}"
-                )
                 redis_node = self.get_redis_connection(target_node)
                 connection = get_connection(redis_node, *args, **kwargs)
                 if asking:
@@ -1114,10 +1098,8 @@ def _execute_command(self, target_node, *args, **kwargs):
                 return response
 
             except (RedisClusterException, BusyLoadingError) as e:
-                log.exception(type(e))
-                raise
+                raise e
             except (ConnectionError, TimeoutError) as e:
-                log.exception(type(e))
                 # ConnectionError can also be raised if we couldn't get a
                 # connection from the pool before timing out, so check that
                 # this is an actual connection before attempting to disconnect.
@@ -1135,7 +1117,7 @@ def _execute_command(self, target_node, *args, **kwargs):
                     # Hard force of reinitialize of the node/slots setup
                     # and try again with the new setup
                     self.nodes_manager.initialize()
-                    raise
+                    raise e
             except MovedError as e:
                 # First, we will try to patch the slots/nodes cache with the
                 # redirected node output and try again. If MovedError exceeds
@@ -1145,7 +1127,6 @@ def _execute_command(self, target_node, *args, **kwargs):
                 # the same client object is shared between multiple threads. To
                 # reduce the frequency you can set this variable in the
                 # RedisCluster constructor.
-                log.exception("MovedError")
                 self.reinitialize_counter += 1
                 if self._should_reinitialized():
                     self.nodes_manager.initialize()
@@ -1155,17 +1136,12 @@ def _execute_command(self, target_node, *args, **kwargs):
                     self.nodes_manager.update_moved_exception(e)
                 moved = True
             except TryAgainError:
-                log.exception("TryAgainError")
-
                 if ttl < self.RedisClusterRequestTTL / 2:
                     time.sleep(0.05)
             except AskError as e:
-                log.exception("AskError")
-
                 redirect_addr = get_node_name(host=e.host, port=e.port)
                 asking = True
             except ClusterDownError as e:
-                log.exception("ClusterDownError")
                 # ClusterDownError can occur during a failover and to get
                 # self-healed, we will try to reinitialize the cluster layout
                 # and retry executing the command
@@ -1173,11 +1149,8 @@ def _execute_command(self, target_node, *args, **kwargs):
                 self.nodes_manager.initialize()
                 raise e
             except ResponseError as e:
-                message = e.__str__()
-                log.exception(f"ResponseError: {message}")
                 raise e
             except BaseException as e:
-                log.exception("BaseException")
                 if connection:
                     connection.disconnect()
                 raise e
@@ -1312,11 +1285,6 @@ def get_node(self, host=None, port=None, node_name=None):
         elif node_name:
             return self.nodes_cache.get(node_name)
         else:
-            log.error(
-                "get_node requires one of the following: "
-                "1. node name "
-                "2. host and port"
-            )
             return None
 
     def update_moved_exception(self, exception):
@@ -1449,7 +1417,6 @@ def initialize(self):
         :startup_nodes:
             Responsible for discovering other nodes in the cluster
         """
-        log.debug("Initializing the nodes' topology of the cluster")
         self.reset()
         tmp_nodes_cache = {}
         tmp_slots = {}
@@ -1477,17 +1444,9 @@ def initialize(self):
                     )
                 cluster_slots = str_if_bytes(r.execute_command("CLUSTER SLOTS"))
                 startup_nodes_reachable = True
-            except (ConnectionError, TimeoutError) as e:
-                msg = e.__str__
-                log.exception(
-                    "An exception occurred while trying to"
-                    " initialize the cluster using the seed node"
-                    f" {startup_node.name}:\n{msg}"
-                )
+            except (ConnectionError, TimeoutError):
                 continue
             except ResponseError as e:
-                log.exception('ReseponseError sending "cluster slots" to redis server')
-
                 # Isn't a cluster connection, so it won't parse these
                 # exceptions automatically
                 message = e.__str__()
@@ -2049,12 +2008,6 @@ def _send_cluster_commands(
             # If a lot of commands have failed, we'll be setting the
             # flag to rebuild the slots table from scratch.
             # So MOVED errors should correct themselves fairly quickly.
-            log.exception(
-                f"An exception occurred during pipeline execution. "
-                f"args: {attempt[-1].args}, "
-                f"error: {type(attempt[-1].result).__name__} "
-                f"{str(attempt[-1].result)}"
-            )
             self.reinitialize_counter += 1
             if self._should_reinitialized():
                 self.nodes_manager.initialize()


Code review comments:
diff_hunk: @@ -1114,10 +1098,8 @@ def _execute_command(self, target_node, *args, **kwargs):
                 return response
 
             except (RedisClusterException, BusyLoadingError) as e:
path: redis/cluster.py
text: @user1:
We can remove this and catch these errors with BaseException as we are not doing anything here