From 79c52af6ce5360a773469a47da86cebb0cafbf3e Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sun, 14 Sep 2025 17:10:33 +0200 Subject: [PATCH] refactor(wait): disallow interval=None in wait functions It is not used internally: we always use an interval to allow for Ctrl-C. It causes ambiguity with 0, which might be a value useful for polling. --- psycopg/psycopg/abc.py | 4 +-- psycopg/psycopg/connection.py | 2 +- psycopg/psycopg/connection_async.py | 2 +- psycopg/psycopg/waiting.py | 44 ++++++++++++++++-------- psycopg_c/psycopg_c/_psycopg/waiting.pyx | 12 +++---- tests/test_waiting.py | 14 +++++++- 6 files changed, 51 insertions(+), 27 deletions(-) diff --git a/psycopg/psycopg/abc.py b/psycopg/psycopg/abc.py index 5af64413d..5fdd1791c 100644 --- a/psycopg/psycopg/abc.py +++ b/psycopg/psycopg/abc.py @@ -57,9 +57,7 @@ class WaitFunc(Protocol): Wait on the connection which generated `PQgen` and return its final result. """ - def __call__( - self, gen: PQGen[RV], fileno: int, interval: float | None = None - ) -> RV: ... + def __call__(self, gen: PQGen[RV], fileno: int, interval: float = ...) -> RV: ... # Adaptation types diff --git a/psycopg/psycopg/connection.py b/psycopg/psycopg/connection.py index 1b2e53886..b38326d83 100644 --- a/psycopg/psycopg/connection.py +++ b/psycopg/psycopg/connection.py @@ -466,7 +466,7 @@ class Connection(BaseConnection[Row]): assert pipeline is self._pipeline self._pipeline = None - def wait(self, gen: PQGen[RV], interval: float | None = _WAIT_INTERVAL) -> RV: + def wait(self, gen: PQGen[RV], interval: float = _WAIT_INTERVAL) -> RV: """ Consume a generator operating on the connection. diff --git a/psycopg/psycopg/connection_async.py b/psycopg/psycopg/connection_async.py index c51a535c3..a12527ab2 100644 --- a/psycopg/psycopg/connection_async.py +++ b/psycopg/psycopg/connection_async.py @@ -501,7 +501,7 @@ class AsyncConnection(BaseConnection[Row]): assert pipeline is self._pipeline self._pipeline = None - async def wait(self, gen: PQGen[RV], interval: float | None = _WAIT_INTERVAL) -> RV: + async def wait(self, gen: PQGen[RV], interval: float = _WAIT_INTERVAL) -> RV: """ Consume a generator operating on the connection. diff --git a/psycopg/psycopg/waiting.py b/psycopg/psycopg/waiting.py index db3845c34..08b3e2f90 100644 --- a/psycopg/psycopg/waiting.py +++ b/psycopg/psycopg/waiting.py @@ -35,7 +35,7 @@ READY_RW = Ready.RW logger = logging.getLogger(__name__) -def wait_selector(gen: PQGen[RV], fileno: int, interval: float | None = None) -> RV: +def wait_selector(gen: PQGen[RV], fileno: int, interval: float = 0.0) -> RV: """ Wait for a generator using the best strategy available. @@ -43,12 +43,14 @@ def wait_selector(gen: PQGen[RV], fileno: int, interval: float | None = None) -> `Ready` values when it would block. :param fileno: the file descriptor to wait on. :param interval: interval (in seconds) to check for other interrupt, e.g. - to allow Ctrl-C. If zero or None, wait indefinitely. + to allow Ctrl-C. :return: whatever `!gen` returns on completion. Consume `!gen`, scheduling `fileno` for completion when it is reported to block. Once ready again send the ready state back to `!gen`. """ + if interval is None: + raise ValueError("indefinite wait not supported anymore") try: s = next(gen) with DefaultSelector() as sel: @@ -68,23 +70,23 @@ def wait_selector(gen: PQGen[RV], fileno: int, interval: float | None = None) -> return rv -def wait_conn(gen: PQGenConn[RV], interval: float | None = None) -> RV: +def wait_conn(gen: PQGenConn[RV], interval: float = 0.0) -> RV: """ Wait for a connection generator using the best strategy available. :param gen: a generator performing database operations and yielding (fd, `Ready`) pairs when it would block. :param interval: interval (in seconds) to check for other interrupt, e.g. - to allow Ctrl-C. If zero or None, wait indefinitely. + to allow Ctrl-C. :return: whatever `!gen` returns on completion. Behave like in `wait()`, but take the fileno to wait from the generator itself, which might change during processing. """ + if interval is None: + raise ValueError("indefinite wait not supported anymore") try: fileno, s = next(gen) - if not interval: - interval = None with DefaultSelector() as sel: sel.register(fileno, s) while True: @@ -102,7 +104,7 @@ def wait_conn(gen: PQGenConn[RV], interval: float | None = None) -> RV: return rv -async def wait_async(gen: PQGen[RV], fileno: int, interval: float | None = None) -> RV: +async def wait_async(gen: PQGen[RV], fileno: int, interval: float = 0.0) -> RV: """ Coroutine waiting for a generator to complete. @@ -110,11 +112,14 @@ async def wait_async(gen: PQGen[RV], fileno: int, interval: float | None = None) `Ready` values when it would block. :param fileno: the file descriptor to wait on. :param interval: interval (in seconds) to check for other interrupt, e.g. - to allow Ctrl-C. If None, wait indefinitely. + to allow Ctrl-C. :return: whatever `!gen` returns on completion. Behave like in `wait()`, but exposing an `asyncio` interface. """ + if interval is None: + raise ValueError("indefinite wait not supported anymore") + # Use an event to block and restart after the fd state changes. # Not sure this is the best implementation but it's a start. ev = Event() @@ -163,19 +168,22 @@ async def wait_async(gen: PQGen[RV], fileno: int, interval: float | None = None) return rv -async def wait_conn_async(gen: PQGenConn[RV], interval: float | None = None) -> RV: +async def wait_conn_async(gen: PQGenConn[RV], interval: float = 0.0) -> RV: """ Coroutine waiting for a connection generator to complete. :param gen: a generator performing database operations and yielding (fd, `Ready`) pairs when it would block. :param interval: interval (in seconds) to check for other interrupt, e.g. - to allow Ctrl-C. If zero or None, wait indefinitely. + to allow Ctrl-C. :return: whatever `!gen` returns on completion. Behave like in `wait()`, but take the fileno to wait from the generator itself, which might change during processing. """ + if interval is None: + raise ValueError("indefinite wait not supported anymore") + # Use an event to block and restart after the fd state changes. # Not sure this is the best implementation but it's a start. ev = Event() @@ -224,12 +232,14 @@ async def wait_conn_async(gen: PQGenConn[RV], interval: float | None = None) -> # Specialised implementation of wait functions. -def wait_select(gen: PQGen[RV], fileno: int, interval: float | None = None) -> RV: +def wait_select(gen: PQGen[RV], fileno: int, interval: float = 0.0) -> RV: """ Wait for a generator using select where supported. BUG: on Linux, can't select on FD >= 1024. On Windows it's fine. """ + if interval is None: + raise ValueError("indefinite wait not supported anymore") try: s = next(gen) @@ -268,7 +278,7 @@ else: _epoll_evmasks = {} -def wait_epoll(gen: PQGen[RV], fileno: int, interval: float | None = None) -> RV: +def wait_epoll(gen: PQGen[RV], fileno: int, interval: float = 0.0) -> RV: """ Wait for a generator using epoll where supported. @@ -284,10 +294,12 @@ def wait_epoll(gen: PQGen[RV], fileno: int, interval: float | None = None) -> RV export PSYCOPG_WAIT_FUNC=wait_epoll pytest tests/test_concurrency.py::test_concurrent_close """ + if interval is None: + raise ValueError("indefinite wait not supported anymore") try: s = next(gen) - if interval is None or interval < 0: + if interval < 0: interval = 0.0 with select.epoll() as epoll: @@ -322,16 +334,18 @@ else: _poll_evmasks = {} -def wait_poll(gen: PQGen[RV], fileno: int, interval: float | None = None) -> RV: +def wait_poll(gen: PQGen[RV], fileno: int, interval: float = 0.0) -> RV: """ Wait for a generator using poll where supported. Parameters are like for `wait()`. """ + if interval is None: + raise ValueError("indefinite wait not supported anymore") try: s = next(gen) - if interval is None or interval < 0: + if interval < 0: interval = 0 else: interval = int(interval * 1000.0) diff --git a/psycopg_c/psycopg_c/_psycopg/waiting.pyx b/psycopg_c/psycopg_c/_psycopg/waiting.pyx index 625b42d5e..03e91bb69 100644 --- a/psycopg_c/psycopg_c/_psycopg/waiting.pyx +++ b/psycopg_c/psycopg_c/_psycopg/waiting.pyx @@ -177,7 +177,7 @@ finally: cdef int wait_c_impl(int fileno, int wait, float timeout) except -1 -def wait_c(gen: PQGen[RV], int fileno, interval = None) -> RV: +def wait_c(gen: PQGen[RV], int fileno, interval = 0.0) -> RV: """ Wait for a generator using poll or select. """ @@ -186,11 +186,11 @@ def wait_c(gen: PQGen[RV], int fileno, interval = None) -> RV: cdef PyObject *pyready if interval is None: - cinterval = -1.0 - else: - cinterval = float(interval) - if cinterval < 0.0: - cinterval = -1.0 + raise ValueError("indefinite wait not supported anymore") + + cinterval = float(interval) + if cinterval < 0.0: + cinterval = 0.0 send = gen.send diff --git a/tests/test_waiting.py b/tests/test_waiting.py index 646f3c5eb..2c40c4767 100644 --- a/tests/test_waiting.py +++ b/tests/test_waiting.py @@ -32,7 +32,7 @@ waitfns = [ events = ["R", "W", "RW"] intervals = [pytest.param({}, id="blank")] -intervals += [pytest.param({"interval": x}, id=str(x)) for x in [None, 0, 0.2, 10]] +intervals += [pytest.param({"interval": x}, id=str(x)) for x in [0, 0.2, 10]] @pytest.mark.parametrize("timeout", intervals) @@ -234,3 +234,15 @@ async def test_wait_async_bad(pgconn): pgconn.finish() with pytest.raises(psycopg.OperationalError): await waiting.wait_async(gen, socket) + + +@pytest.mark.parametrize("waitfn", waitfns) +def test_wait_timeout_none_unsupported(waitfn): + waitfn = getattr(waiting, waitfn) + + def gen(): + r = yield waiting.Wait.R + return r + + with pytest.raises(ValueError): + waitfn(gen(), 1, None) -- 2.47.3