From: Denis Laxalde <denis.laxalde@dalibo.com>
Date: Tue, 16 Nov 2021 09:55:26 +0000 (+0100)
Subject: Disallow pool re-opening
X-Git-Tag: pool-3.1~45^2~5
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b95797af9acba8126647a1cc84fd71b1758954d9;p=thirdparty%2Fpsycopg.git

Disallow pool re-opening

There seems to be no use case for re-opening a closed pool, so disable
this now (until we find a real use-case).
---

diff --git a/docs/news_pool.rst b/docs/news_pool.rst
index 62c6a3355..08a1b70e0 100644
--- a/docs/news_pool.rst
+++ b/docs/news_pool.rst
@@ -15,6 +15,8 @@ psycopg_pool 3.1.0
 
 - Add `ConnectionPool.open()` and `AsyncConnectionPool.open()`
   (:ticket:`#155`).
+- Raise an `~psycopg.OperationalError` when trying to re-open a closed pool
+  (:ticket:`#155`).
 
 psycopg_pool 3.0.2
 ^^^^^^^^^^^^^^^^^^
diff --git a/psycopg_pool/psycopg_pool/base.py b/psycopg_pool/psycopg_pool/base.py
index 129ffc623..8c1b66849 100644
--- a/psycopg_pool/psycopg_pool/base.py
+++ b/psycopg_pool/psycopg_pool/base.py
@@ -8,6 +8,7 @@ from random import random
 from typing import Any, Callable, Dict, Generic, Optional
 
 from psycopg.abc import ConnectionType
+from psycopg import errors as e
 
 from ._compat import Counter, Deque
 
@@ -94,6 +95,7 @@ class BasePool(Generic[ConnectionType]):
         # connections to the pool.
         self._growing = False
 
+        self._opened = False
         self._closed = True
 
     def __repr__(self) -> str:
@@ -115,6 +117,12 @@ class BasePool(Generic[ConnectionType]):
         """`!True` if the pool is closed."""
         return self._closed
 
+    def _check_open(self) -> None:
+        if self._closed and self._opened:
+            raise e.OperationalError(
+                "pool has already been opened/closed and cannot be reused"
+            )
+
     def get_stats(self) -> Dict[str, int]:
         """
         Return current stats about the pool usage.
diff --git a/psycopg_pool/psycopg_pool/pool.py b/psycopg_pool/psycopg_pool/pool.py
index f92de0bed..71479d41b 100644
--- a/psycopg_pool/psycopg_pool/pool.py
+++ b/psycopg_pool/psycopg_pool/pool.py
@@ -231,6 +231,8 @@ class ConnectionPool(BasePool[Connection[Any]]):
         if not self._closed:
             return
 
+        self._check_open()
+
         self._sched_runner = threading.Thread(
             target=self._sched.run, name=f"{self.name}-scheduler", daemon=True
         )
@@ -258,6 +260,7 @@ class ConnectionPool(BasePool[Connection[Any]]):
         self.schedule_task(ShrinkPool(self), self.max_idle)
 
         self._closed = False
+        self._opened = True
 
     def close(self, timeout: float = 5.0) -> None:
         """Close the pool and make it unavailable to new clients.
diff --git a/psycopg_pool/psycopg_pool/pool_async.py b/psycopg_pool/psycopg_pool/pool_async.py
index c954898b1..46a3bd062 100644
--- a/psycopg_pool/psycopg_pool/pool_async.py
+++ b/psycopg_pool/psycopg_pool/pool_async.py
@@ -198,6 +198,8 @@ class AsyncConnectionPool(BasePool[AsyncConnection[Any]]):
         if not self._closed:
             return
 
+        self._check_open()
+
         self._sched_runner = create_task(
             self._sched.run(), name=f"{self.name}-scheduler"
         )
@@ -217,6 +219,7 @@ class AsyncConnectionPool(BasePool[AsyncConnection[Any]]):
         self.run_task(Schedule(self, ShrinkPool(self), self.max_idle))
 
         self._closed = False
+        self._opened = True
 
     async def close(self, timeout: float = 5.0) -> None:
         if self._closed:
diff --git a/tests/pool/test_pool.py b/tests/pool/test_pool.py
index 47f90ee5d..ba2812b58 100644
--- a/tests/pool/test_pool.py
+++ b/tests/pool/test_pool.py
@@ -8,6 +8,7 @@ from typing import Any, List, Tuple
 import pytest
 
 import psycopg
+from psycopg.errors import OperationalError
 from psycopg.pq import TransactionStatus
 from psycopg._compat import Counter
 
@@ -690,12 +691,9 @@ def test_reopen(dsn):
     p.close()
     assert p._sched_runner is None
     assert not p._workers
-    p.open()
-    assert p._sched_runner is not None
-    assert p._workers
-    with p.connection() as conn:
-        conn.execute("select 1")
-    p.close()
+
+    with pytest.raises(OperationalError, match="cannot be reused"):
+        p.open()
 
 
 @pytest.mark.slow
diff --git a/tests/pool/test_pool_async.py b/tests/pool/test_pool_async.py
index fb12ecbf8..7c2a9ae9f 100644
--- a/tests/pool/test_pool_async.py
+++ b/tests/pool/test_pool_async.py
@@ -7,6 +7,7 @@ from typing import Any, List, Tuple
 import pytest
 
 import psycopg
+from psycopg.errors import OperationalError
 from psycopg.pq import TransactionStatus
 from psycopg._compat import create_task, Counter
 
@@ -678,11 +679,9 @@ async def test_reopen(dsn):
         await conn.execute("select 1")
     await p.close()
     assert p._sched_runner is None
-    p.open()
-    assert p._sched_runner is not None
-    async with p.connection() as conn:
-        await conn.execute("select 1")
-    await p.close()
+
+    with pytest.raises(OperationalError, match="cannot be reused"):
+        p.open()
 
 
 @pytest.mark.slow