Remove use of deprecated argument (cluster_error_retry_attempts) in RedisCluster.pipeline construction of ClusterPipeline

[alisaifee]

Pull Request check-list

Please make sure to review and check all of these items:

• Do tests and lints pass with this change?
• 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?

Description of change

cluster_error_retry_attempts is a deprecated argument that was being passed to the constructor of ClusterPipeline from the pipeline method and thus using the cluster pipeline instance by way of constructing it through the pipeline() method was logging a deprecation warning.

Since the value is already being derived from self.retry and retry=self.retry is already being passed as an argument it should be safe to remove it without any change in behaviour.

Example:

bash-3.2$ python -W error -c 'import redis; redis.RedisCluster.from_url("redis://localhost:7001").pipeline()'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/Users/ali/_dev/_git/redis-py/redis/cluster.py", line 853, in pipeline
    return ClusterPipeline(
           ^^^^^^^^^^^^^^^^
  File "/Users/ali/_dev/_git/redis-py/redis/utils.py", line 187, in wrapper
    warn_deprecated_arg_usage(
  File "/Users/ali/_dev/_git/redis-py/redis/utils.py", line 150, in warn_deprecated_arg_usage
    warnings.warn(msg, category=DeprecationWarning, stacklevel=stacklevel)
DeprecationWarning: Call to '__init__' function with deprecated usage of input argument/s 'cluster_error_retry_attempts'. (Please configure the 'retry' object instead) -- Deprecated since version 6.0.0.

[petyaslavova]

Hi @alisaifee, thank you for your contribution! Unfortunately we can't remove the argument yet. It has been deprecated in 6.0 and we need to keep it at least for three major releases before completely removing it.

[alisaifee]

Unfortunately we can't remove the argument yet.

Hi, @petyaslavova - sorry for the confusion, the PR isn't removing cluster_error_retry_attempts from the definition of ClusterPipeline, it is simply removing the use of it in the RedisCluster.pipeline where the cluster pipeline is currently being constructed as:

return ClusterPipeline(
    nodes_manager=self.nodes_manager,
    commands_parser=self.commands_parser,
    startup_nodes=self.nodes_manager.startup_nodes,
    result_callbacks=self.result_callbacks,
    cluster_response_callbacks=self.cluster_response_callbacks,
    cluster_error_retry_attempts=self.retry.get_retries(),
    read_from_replicas=self.read_from_replicas,
    load_balancing_strategy=self.load_balancing_strategy,
    reinitialize_steps=self.reinitialize_steps,
    retry=self.retry,
    lock=self._lock,
    transaction=transaction,
)

As you can see the assignment of cluster_error_retry_attempt derives it's value from self.retry.get_retries() as well as assigning retry=self.retry. This effectively means that the assignment in this context has no effect since the constructor of ClusterPipeline only considers it when retry is None:

if retry is not None:
    self.retry = retry
else:
    self.retry = Retry(
        backoff=ExponentialWithJitterBackoff(base=1, cap=10),
        retries=cluster_error_retry_attempts,
        )

For reference, code coverage also shows that this path is not exercised.