From: Daniele Varrazzo <daniele.varrazzo@gmail.com>
Date: Sun, 8 Oct 2023 18:43:47 +0000 (+0200)
Subject: feat(pool): raise a warning if async pools are open in the constructor
X-Git-Tag: pool-3.2.0~8^2~1
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=b427b9efe182887c6247893676fdc82f1989c77b;p=thirdparty%2Fpsycopg.git

feat(pool): raise a warning if async pools are open in the constructor
---

diff --git a/docs/news_pool.rst b/docs/news_pool.rst
index 7cc79a815..636dc9a8c 100644
--- a/docs/news_pool.rst
+++ b/docs/news_pool.rst
@@ -16,9 +16,11 @@ psycopg_pool 3.2.0 (unreleased)
 - Add support for async `!reconnect_failed` callbacks in `AsyncConnectionPool`
   (:ticket:`#520`).
 - Make connection pool classes generic on the connection type (:ticket:`#559`).
-- Raise a warning if a pool is used relying on an implicit `!open=True` and the
+- Raise a warning if sync pools rely an implicit `!open=True` and the
   pool context is not used. In the future the default will become `!False`
   (:ticket:`#659`).
+- Raise a warning if async pools are opened in the constructor. In the future
+  it will become an error. (:ticket:`#659`).
 
 
 psycopg_pool 3.1.9 (unreleased)
diff --git a/psycopg_pool/psycopg_pool/base.py b/psycopg_pool/psycopg_pool/base.py
index a14f7a2d6..1ed6ab1c9 100644
--- a/psycopg_pool/psycopg_pool/base.py
+++ b/psycopg_pool/psycopg_pool/base.py
@@ -7,7 +7,6 @@ psycopg connection pool base class and functionalities.
 from time import monotonic
 from random import random
 from typing import Any, Dict, Optional, Tuple, TYPE_CHECKING
-import warnings
 
 from psycopg import errors as e
 
@@ -142,13 +141,6 @@ class BasePool:
                 raise PoolClosed(f"the pool {self.name!r} is already closed")
             else:
                 raise PoolClosed(f"the pool {self.name!r} is not open yet")
-        elif self._open_implicit:
-            warnings.warn(
-                f"the default for the {type(self).__name__} 'open' parameter will"
-                " become 'False' in a future release; please use open={True|False}"
-                " explicitly or use the pool as context manager",
-                DeprecationWarning,
-            )
 
     def _check_pool_putconn(self, conn: "BaseConnection[Any]") -> None:
         pool = getattr(conn, "_pool", None)
diff --git a/psycopg_pool/psycopg_pool/pool.py b/psycopg_pool/psycopg_pool/pool.py
index 8abecd6ed..185cef08d 100644
--- a/psycopg_pool/psycopg_pool/pool.py
+++ b/psycopg_pool/psycopg_pool/pool.py
@@ -10,6 +10,7 @@ Psycopg connection pool module (sync version).
 from __future__ import annotations
 
 import logging
+import warnings
 from abc import ABC, abstractmethod
 from time import monotonic
 from types import TracebackType
@@ -152,6 +153,20 @@ class ConnectionPool(Generic[CT], BasePool):
 
         self._stop_workers()
 
+    def _check_open_getconn(self) -> None:
+        super()._check_open_getconn()
+
+        if self._open_implicit:
+            self._open_implicit = False
+
+            warnings.warn(
+                f"the default for the {type(self).__name__} 'open' parameter"
+                + " will become 'False' in a future release. Please use"
+                + " open={True|False} explicitly, or use the pool as context"
+                + f" manager using: `with {type(self).__name__}(...) as pool: ...`",
+                DeprecationWarning,
+            )
+
     def wait(self, timeout: float = 30.0) -> None:
         """
         Wait for the pool to be full (with `min_size` connections) after creation.
diff --git a/psycopg_pool/psycopg_pool/pool_async.py b/psycopg_pool/psycopg_pool/pool_async.py
index 2077b425b..c1c371022 100644
--- a/psycopg_pool/psycopg_pool/pool_async.py
+++ b/psycopg_pool/psycopg_pool/pool_async.py
@@ -7,6 +7,7 @@ Psycopg connection pool module (async version).
 from __future__ import annotations
 
 import logging
+import warnings
 from abc import ABC, abstractmethod
 from time import monotonic
 from types import TracebackType
@@ -137,6 +138,10 @@ class AsyncConnectionPool(Generic[ACT], BasePool):
             num_workers=num_workers,
         )
 
+        if True:  # ASYNC
+            if open:
+                self._warn_open_async()
+
         if open is None:
             open = self._open_implicit = True
 
@@ -153,6 +158,35 @@ class AsyncConnectionPool(Generic[ACT], BasePool):
 
             self._stop_workers()
 
+    def _check_open_getconn(self) -> None:
+        super()._check_open_getconn()
+
+        if self._open_implicit:
+            self._open_implicit = False
+
+            if True:  # ASYNC
+                # If open was explicit, we already warned it in __init__
+                self._warn_open_async()
+            else:
+                warnings.warn(
+                    f"the default for the {type(self).__name__} 'open' parameter"
+                    + " will become 'False' in a future release. Please use"
+                    + " open={True|False} explicitly, or use the pool as context"
+                    + f" manager using: `with {type(self).__name__}(...) as pool: ...`",
+                    DeprecationWarning,
+                )
+
+    if True:  # ASYNC
+
+        def _warn_open_async(self) -> None:
+            warnings.warn(
+                f"opening the async pool {type(self).__name__} in the constructor"
+                " is deprecated and will not be supported anymore in a future"
+                " release. Please use `await pool.open()`, or use the pool as context"
+                f" manager using: `async with {type(self).__name__}(...) as pool: `...",
+                DeprecationWarning,
+            )
+
     async def wait(self, timeout: float = 30.0) -> None:
         """
         Wait for the pool to be full (with `min_size` connections) after creation.
diff --git a/tests/pool/test_pool.py b/tests/pool/test_pool.py
index f16d5480b..fd3d656b3 100644
--- a/tests/pool/test_pool.py
+++ b/tests/pool/test_pool.py
@@ -359,7 +359,8 @@ def test_fail_rollback_close(dsn, caplog, monkeypatch):
 
 
 def test_del_no_warning(dsn, recwarn):
-    p = pool.ConnectionPool(dsn, min_size=2, open=True)
+    p = pool.ConnectionPool(dsn, min_size=2, open=False)
+    p.open()
     with p.connection() as conn:
         conn.execute("select 1")
 
@@ -788,7 +789,7 @@ def test_debug_deadlock(dsn):
     handler.setLevel(logging.DEBUG)
     logger.addHandler(handler)
     try:
-        with pool.ConnectionPool(dsn, min_size=4, open=True) as p:
+        with pool.ConnectionPool(dsn, min_size=4) as p:
             p.wait(timeout=2)
     finally:
         logger.removeHandler(handler)
diff --git a/tests/pool/test_pool_async.py b/tests/pool/test_pool_async.py
index a49622062..195fec818 100644
--- a/tests/pool/test_pool_async.py
+++ b/tests/pool/test_pool_async.py
@@ -363,7 +363,8 @@ async def test_fail_rollback_close(dsn, caplog, monkeypatch):
 
 
 async def test_del_no_warning(dsn, recwarn):
-    p = pool.AsyncConnectionPool(dsn, min_size=2, open=True)
+    p = pool.AsyncConnectionPool(dsn, min_size=2, open=False)
+    await p.open()
     async with p.connection() as conn:
         await conn.execute("select 1")
 
@@ -794,7 +795,7 @@ async def test_debug_deadlock(dsn):
     handler.setLevel(logging.DEBUG)
     logger.addHandler(handler)
     try:
-        async with pool.AsyncConnectionPool(dsn, min_size=4, open=True) as p:
+        async with pool.AsyncConnectionPool(dsn, min_size=4) as p:
             await p.wait(timeout=2)
     finally:
         logger.removeHandler(handler)
diff --git a/tests/pool/test_pool_async_noasyncio.py b/tests/pool/test_pool_async_noasyncio.py
index 982b9288e..304aa4ceb 100644
--- a/tests/pool/test_pool_async_noasyncio.py
+++ b/tests/pool/test_pool_async_noasyncio.py
@@ -56,8 +56,9 @@ def test_working_created_before_loop(dsn, asyncio_run):
 
 
 def test_cant_create_open_outside_loop(dsn):
-    with pytest.raises(RuntimeError):
-        pool.AsyncConnectionPool(dsn, open=True)
+    with pytest.warns(DeprecationWarning):
+        with pytest.raises(RuntimeError):
+            pool.AsyncConnectionPool(dsn, open=True)
 
 
 @pytest.fixture
diff --git a/tests/pool/test_pool_common.py b/tests/pool/test_pool_common.py
index e12433786..df76266a5 100644
--- a/tests/pool/test_pool_common.py
+++ b/tests/pool/test_pool_common.py
@@ -55,7 +55,7 @@ def test_context(pool_cls, dsn):
 
 
 def test_create_warning(pool_cls, dsn):
-    # No warning on explicit open
+    # No warning on explicit open for sync pool
     p = pool_cls(dsn, open=True)
     try:
         with p.connection():
@@ -357,7 +357,8 @@ def test_del_stops_threads(pool_cls, dsn):
 
 
 def test_closed_getconn(pool_cls, dsn):
-    p = pool_cls(dsn, min_size=min_size(pool_cls), open=True)
+    p = pool_cls(dsn, min_size=min_size(pool_cls), open=False)
+    p.open()
     assert not p.closed
     with p.connection():
         pass
@@ -371,7 +372,8 @@ def test_closed_getconn(pool_cls, dsn):
 
 
 def test_close_connection_on_pool_close(pool_cls, dsn):
-    p = pool_cls(dsn, min_size=min_size(pool_cls), open=True)
+    p = pool_cls(dsn, min_size=min_size(pool_cls), open=False)
+    p.open()
     with p.connection() as conn:
         p.close()
     assert conn.closed
@@ -396,18 +398,17 @@ def test_closed_queue(pool_cls, dsn):
     e1 = Event()
     e2 = Event()
 
-    p = pool_cls(dsn, min_size=min_size(pool_cls), max_size=1, open=True)
-    p.wait()
-    success: List[str] = []
+    with pool_cls(dsn, min_size=min_size(pool_cls), max_size=1) as p:
+        p.wait()
+        success: List[str] = []
 
-    t1 = spawn(w1)
-    # Wait until w1 has received a connection
-    e1.wait()
+        t1 = spawn(w1)
+        # Wait until w1 has received a connection
+        e1.wait()
 
-    t2 = spawn(w2)
-    # Wait until w2 is in the queue
-    ensure_waiting(p)
-    p.close()
+        t2 = spawn(w2)
+        # Wait until w2 is in the queue
+        ensure_waiting(p)
 
     # Wait for the workers to finish
     e2.set()
@@ -454,7 +455,8 @@ def test_open_context(pool_cls, dsn):
 
 
 def test_open_no_op(pool_cls, dsn):
-    p = pool_cls(dsn, open=True)
+    p = pool_cls(dsn, open=False)
+    p.open()
     try:
         assert not p.closed
         p.open()
@@ -468,7 +470,8 @@ def test_open_no_op(pool_cls, dsn):
 
 
 def test_reopen(pool_cls, dsn):
-    p = pool_cls(dsn, open=True)
+    p = pool_cls(dsn, open=False)
+    p.open()
     with p.connection() as conn:
         conn.execute("select 1")
     p.close()
@@ -567,7 +570,7 @@ def test_debug_deadlock(pool_cls, dsn):
     handler.setLevel(logging.DEBUG)
     logger.addHandler(handler)
     try:
-        with pool_cls(dsn, min_size=min_size(pool_cls, 4), open=True) as p:
+        with pool_cls(dsn, min_size=min_size(pool_cls, 4)) as p:
             p.wait(timeout=2)
     finally:
         logger.removeHandler(handler)
diff --git a/tests/pool/test_pool_common_async.py b/tests/pool/test_pool_common_async.py
index b368fab54..f23165253 100644
--- a/tests/pool/test_pool_common_async.py
+++ b/tests/pool/test_pool_common_async.py
@@ -57,13 +57,20 @@ async def test_context(pool_cls, dsn):
 
 
 async def test_create_warning(pool_cls, dsn):
-    # No warning on explicit open
-    p = pool_cls(dsn, open=True)
-    try:
-        async with p.connection():
-            pass
-    finally:
-        await p.close()
+    if True:  # ASYNC
+        # warning on explicit open too on async
+        with pytest.warns(DeprecationWarning):
+            p = pool_cls(dsn, open=True)
+            await p.close()
+
+    else:
+        # No warning on explicit open for sync pool
+        p = pool_cls(dsn, open=True)
+        try:
+            async with p.connection():
+                pass
+        finally:
+            await p.close()
 
     # No warning on explicit close
     p = pool_cls(dsn, open=False)
@@ -367,7 +374,8 @@ async def test_del_stops_threads(pool_cls, dsn):
 
 
 async def test_closed_getconn(pool_cls, dsn):
-    p = pool_cls(dsn, min_size=min_size(pool_cls), open=True)
+    p = pool_cls(dsn, min_size=min_size(pool_cls), open=False)
+    await p.open()
     assert not p.closed
     async with p.connection():
         pass
@@ -381,7 +389,8 @@ async def test_closed_getconn(pool_cls, dsn):
 
 
 async def test_close_connection_on_pool_close(pool_cls, dsn):
-    p = pool_cls(dsn, min_size=min_size(pool_cls), open=True)
+    p = pool_cls(dsn, min_size=min_size(pool_cls), open=False)
+    await p.open()
     async with p.connection() as conn:
         await p.close()
     assert conn.closed
@@ -406,18 +415,17 @@ async def test_closed_queue(pool_cls, dsn):
     e1 = AEvent()
     e2 = AEvent()
 
-    p = pool_cls(dsn, min_size=min_size(pool_cls), max_size=1, open=True)
-    await p.wait()
-    success: List[str] = []
+    async with pool_cls(dsn, min_size=min_size(pool_cls), max_size=1) as p:
+        await p.wait()
+        success: List[str] = []
 
-    t1 = spawn(w1)
-    # Wait until w1 has received a connection
-    await e1.wait()
+        t1 = spawn(w1)
+        # Wait until w1 has received a connection
+        await e1.wait()
 
-    t2 = spawn(w2)
-    # Wait until w2 is in the queue
-    await ensure_waiting(p)
-    await p.close()
+        t2 = spawn(w2)
+        # Wait until w2 is in the queue
+        await ensure_waiting(p)
 
     # Wait for the workers to finish
     e2.set()
@@ -465,7 +473,8 @@ async def test_open_context(pool_cls, dsn):
 
 
 async def test_open_no_op(pool_cls, dsn):
-    p = pool_cls(dsn, open=True)
+    p = pool_cls(dsn, open=False)
+    await p.open()
     try:
         assert not p.closed
         await p.open()
@@ -480,7 +489,8 @@ async def test_open_no_op(pool_cls, dsn):
 
 
 async def test_reopen(pool_cls, dsn):
-    p = pool_cls(dsn, open=True)
+    p = pool_cls(dsn, open=False)
+    await p.open()
     async with p.connection() as conn:
         await conn.execute("select 1")
     await p.close()
@@ -579,7 +589,7 @@ async def test_debug_deadlock(pool_cls, dsn):
     handler.setLevel(logging.DEBUG)
     logger.addHandler(handler)
     try:
-        async with pool_cls(dsn, min_size=min_size(pool_cls, 4), open=True) as p:
+        async with pool_cls(dsn, min_size=min_size(pool_cls, 4)) as p:
             await p.wait(timeout=2)
     finally:
         logger.removeHandler(handler)