Repository: redis/redis-py
Pull request #2900 (2023-08-20 11:38:04)

Fix: automatically close connection pool for async Sentinel
### Pull Request check-list

_Please make sure to review and check all of these items:_

- [ ] Does `$ tox` pass with this change (including linting)?
- [ ] Do the CI tests pass with this change (enable it first in your forked repo and wait for the github action build to finish)?
- [x] 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)?
- [ ] 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

A `Redis` object returned by `redis.asyncio.sentinel.Sentinel()` has its _own_ `ConnectionPool`, but this is not closed automatically when the `Redis` object goes out of scope.

By setting the `auto_close_connection_pool` Redis attribute, it is ensured that connections are closed properly.

This fixes another issue raised in #2831 

_Please provide a description of the change here._


codecov-commenter: ## [Codecov](https://app.codecov.io/gh/redis/redis-py/pull/2900?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis) Report
Patch coverage: **`16.00%`** and project coverage change: **`-24.85%`** :warning:
> Comparison is base [(`6968431`)](https://app.codecov.io/gh/redis/redis-py/commit/6968431983c20a6f874a97d92c4d11366721b83b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis) 91.27% compared to head [(`0d17f06`)](https://app.codecov.io/gh/redis/redis-py/pull/2900?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis) 66.43%.

:exclamation: Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please [install the Github App Integration](https://github.com/apps/codecov) for your organization. [Read more](https://about.codecov.io/blog/codecov-is-updating-its-github-integration/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis).

<details><summary>Additional details and impacted files</summary>


```diff
@@             Coverage Diff             @@
##           master    #2900       +/-   ##
===========================================
- Coverage   91.27%   66.43%   -24.85%     
===========================================
  Files         126      126               
  Lines       32368    32391       +23     
===========================================
- Hits        29545    21518     -8027     
- Misses       2823    10873     +8050     
```


| [Files Changed](https://app.codecov.io/gh/redis/redis-py/pull/2900?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis) | Coverage Δ | |
|---|---|---|
| [redis/asyncio/sentinel.py](https://app.codecov.io/gh/redis/redis-py/pull/2900?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis#diff-cmVkaXMvYXN5bmNpby9zZW50aW5lbC5weQ==) | `29.88% <0.00%> (-56.87%)` | :arrow_down: |
| [tests/test\_asyncio/test\_sentinel.py](https://app.codecov.io/gh/redis/redis-py/pull/2900?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis#diff-dGVzdHMvdGVzdF9hc3luY2lvL3Rlc3Rfc2VudGluZWwucHk=) | `35.22% <26.66%> (-64.09%)` | :arrow_down: |

... and [80 files with indirect coverage changes](https://app.codecov.io/gh/redis/redis-py/pull/2900/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis)


</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/redis/redis-py/pull/2900?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis).


Patch:
diff --git a/redis/asyncio/sentinel.py b/redis/asyncio/sentinel.py
index 501e234c..5ed92409 100644
--- a/redis/asyncio/sentinel.py
+++ b/redis/asyncio/sentinel.py
@@ -335,11 +335,15 @@ class Sentinel(AsyncSentinelCommands):
         kwargs["is_master"] = True
         connection_kwargs = dict(self.connection_kwargs)
         connection_kwargs.update(kwargs)
-        return redis_class(
-            connection_pool=connection_pool_class(
-                service_name, self, **connection_kwargs
-            )
+
+        connection_pool = connection_pool_class(service_name, self, **connection_kwargs)
+        # The Redis object "owns" the pool
+        auto_close_connection_pool = True
+        client = redis_class(
+            connection_pool=connection_pool,
         )
+        client.auto_close_connection_pool = auto_close_connection_pool
+        return client
 
     def slave_for(
         self,
@@ -368,8 +372,12 @@ class Sentinel(AsyncSentinelCommands):
         kwargs["is_master"] = False
         connection_kwargs = dict(self.connection_kwargs)
         connection_kwargs.update(kwargs)
-        return redis_class(
-            connection_pool=connection_pool_class(
-                service_name, self, **connection_kwargs
-            )
+
+        connection_pool = connection_pool_class(service_name, self, **connection_kwargs)
+        # The Redis object "owns" the pool
+        auto_close_connection_pool = True
+        client = redis_class(
+            connection_pool=connection_pool,
         )
+        client.auto_close_connection_pool = auto_close_connection_pool
+        return client


Test patch:
diff --git a/tests/test_asyncio/test_sentinel.py b/tests/test_asyncio/test_sentinel.py
index 2091f2cb..a2d52f17 100644
--- a/tests/test_asyncio/test_sentinel.py
+++ b/tests/test_asyncio/test_sentinel.py
@@ -1,4 +1,5 @@
 import socket
+from unittest import mock
 
 import pytest
 import pytest_asyncio
@@ -239,3 +240,28 @@ async def test_flushconfig(cluster, sentinel):
 async def test_reset(cluster, sentinel):
     cluster.master["is_odown"] = True
     assert await sentinel.sentinel_reset("mymaster")
+
+
+@pytest.mark.onlynoncluster
+@pytest.mark.parametrize("method_name", ["master_for", "slave_for"])
+async def test_auto_close_pool(cluster, sentinel, method_name):
+    """
+    Check that the connection pool created by the sentinel client is
+    automatically closed
+    """
+
+    method = getattr(sentinel, method_name)
+    client = method("mymaster", db=9)
+    pool = client.connection_pool
+    assert client.auto_close_connection_pool is True
+    calls = 0
+
+    async def mock_disconnect():
+        nonlocal calls
+        calls += 1
+
+    with mock.patch.object(pool, "disconnect", mock_disconnect):
+        await client.close()
+
+    assert calls == 1
+    await pool.disconnect()