fix: Add warning if relying on Redis max_connections parameter

The `new_feature_store` and `new_big_segment_store` methods accept a
`max_connections` parameter that has been unused since v8.

We cannot start using this parameter now as it would be a functionally
breaking change. Instead, we are opting to warn customers who might try
setting it explicitly.

fixes #386

Author: keelerm84

ldclient/integrations/__init__.py

@@ -6,6 +6,7 @@
 from threading import Event
 from typing import Any, Callable, Dict, List, Mapping, Optional
 
+from ldclient import log
 from ldclient.config import Builder, Config
 from ldclient.feature_store import CacheConfig
 from ldclient.feature_store_helpers import CachingStoreWrapper
@@ -168,12 +169,21 @@ def new_feature_store(
         :param url: the URL of the Redis host; defaults to ``DEFAULT_URL``
         :param prefix: a namespace prefix to be prepended to all Redis keys; defaults to
           ``DEFAULT_PREFIX``
+        :param max_connections: (deprecated and unused) This parameter is not used. To configure
+          the maximum number of connections, use ``redis_opts={'max_connections': N}`` instead.
         :param caching: specifies whether local caching should be enabled and if so,
           sets the cache properties; defaults to :func:`ldclient.feature_store.CacheConfig.default()`
         :param redis_opts: extra options for initializing Redis connection from the url,
           see `redis.connection.ConnectionPool.from_url` for more details.
         """
 
+        if max_connections != Redis.DEFAULT_MAX_CONNECTIONS:
+            log.warning(
+                "The max_connections parameter is not used and will be removed in a future version. "
+                "Please set max_connections in redis_opts instead, e.g., redis_opts={'max_connections': %d}",
+                max_connections
+            )
+
         core = _RedisFeatureStoreCore(url, prefix, redis_opts)
         wrapper = CachingStoreWrapper(core, caching)
         wrapper._core = core  # exposed for testing
@@ -200,10 +210,19 @@ def new_big_segment_store(url: str = 'redis://localhost:6379/0', prefix: str = '
         :param url: the URL of the Redis host; defaults to ``DEFAULT_URL``
         :param prefix: a namespace prefix to be prepended to all Redis keys; defaults to
           ``DEFAULT_PREFIX``
+        :param max_connections: (deprecated and unused) This parameter is not used. To configure
+          the maximum number of connections, use ``redis_opts={'max_connections': N}`` instead.
         :param redis_opts: extra options for initializing Redis connection from the url,
           see `redis.connection.ConnectionPool.from_url` for more details.
         """
 
+        if max_connections != Redis.DEFAULT_MAX_CONNECTIONS:
+            log.warning(
+                "The max_connections parameter is not used and will be removed in a future version. "
+                "Please set max_connections in redis_opts instead, e.g., redis_opts={'max_connections': %d}",
+                max_connections
+            )
+
         return _RedisBigSegmentStore(url, prefix, redis_opts)
 
 

ldclient/testing/integrations/test_redis.py

@@ -125,3 +125,85 @@ class TestRedisBigSegmentStore(BigSegmentStoreTestBase):
     @property
     def tester_class(self):
         return RedisBigSegmentStoreTester
+
+
+@pytest.mark.skipif(skip_database_tests, reason="skipping database tests")
+def test_feature_store_max_connections_is_not_used():
+    """Test that the max_connections parameter is NOT passed to the Redis connection pool."""
+    custom_max_connections = 42
+    store = Redis.new_feature_store(max_connections=custom_max_connections)
+
+    # Access the connection pool through the wrapper's core
+    actual_max_connections = store._core._pool.max_connections
+
+    # Should NOT be our custom value since the parameter is unused
+    assert actual_max_connections != custom_max_connections, \
+        f"Expected max_connections to NOT be {custom_max_connections}, but it was set"
+
+
+@pytest.mark.skipif(skip_database_tests, reason="skipping database tests")
+def test_big_segment_store_max_connections_is_not_used():
+    """Test that the max_connections parameter is NOT passed to the Redis connection pool."""
+    custom_max_connections = 42
+    store = Redis.new_big_segment_store(max_connections=custom_max_connections)
+
+    # Access the connection pool directly from the store
+    actual_max_connections = store._pool.max_connections
+
+    # Should NOT be our custom value since the parameter is unused
+    assert actual_max_connections != custom_max_connections, \
+        f"Expected max_connections to NOT be {custom_max_connections}, but it was set"
+
+
+@pytest.mark.skipif(skip_database_tests, reason="skipping database tests")
+def test_feature_store_max_connections_warns_when_non_default(caplog):
+    """Test that a warning is logged when max_connections differs from the default."""
+    import logging
+    caplog.set_level(logging.WARNING)
+
+    custom_max_connections = 42
+    Redis.new_feature_store(max_connections=custom_max_connections)
+
+    assert any("max_connections parameter is not used" in record.message for record in caplog.records), \
+        "Expected warning that parameter is not used"
+    assert any("redis_opts" in record.message for record in caplog.records), \
+        "Expected warning to mention redis_opts"
+
+
+@pytest.mark.skipif(skip_database_tests, reason="skipping database tests")
+def test_big_segment_store_max_connections_warns_when_non_default(caplog):
+    """Test that a warning is logged when max_connections differs from the default."""
+    import logging
+    caplog.set_level(logging.WARNING)
+
+    custom_max_connections = 42
+    Redis.new_big_segment_store(max_connections=custom_max_connections)
+
+    assert any("max_connections parameter is not used" in record.message for record in caplog.records), \
+        "Expected warning that parameter is not used"
+    assert any("redis_opts" in record.message for record in caplog.records), \
+        "Expected warning to mention redis_opts"
+
+
+@pytest.mark.skipif(skip_database_tests, reason="skipping database tests")
+def test_feature_store_max_connections_no_warn_when_default(caplog):
+    """Test that no warning is logged when max_connections is the default value."""
+    import logging
+    caplog.set_level(logging.WARNING)
+
+    Redis.new_feature_store(max_connections=Redis.DEFAULT_MAX_CONNECTIONS)
+
+    assert not any("max_connections parameter is not used" in record.message for record in caplog.records), \
+        "Expected no warning when using default value"
+
+
+@pytest.mark.skipif(skip_database_tests, reason="skipping database tests")
+def test_big_segment_store_max_connections_no_warn_when_default(caplog):
+    """Test that no warning is logged when max_connections is the default value."""
+    import logging
+    caplog.set_level(logging.WARNING)
+
+    Redis.new_big_segment_store(max_connections=Redis.DEFAULT_MAX_CONNECTIONS)
+
+    assert not any("max_connections parameter is not used" in record.message for record in caplog.records), \
+        "Expected no warning when using default value"