From dea2e795c20c101fe85863e8fbf9c182432fdaca Mon Sep 17 00:00:00 2001 From: jakkdl Date: Wed, 22 May 2024 17:52:50 +0200 Subject: [PATCH 01/24] flake8-trio has been renamed --- docs/source/awesome-trio-libraries.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/source/awesome-trio-libraries.rst b/docs/source/awesome-trio-libraries.rst index eb302550c4..823bf0779a 100644 --- a/docs/source/awesome-trio-libraries.rst +++ b/docs/source/awesome-trio-libraries.rst @@ -101,7 +101,7 @@ Testing Tools and Utilities ------------------- * `trio-util `__ - An assortment of utilities for the Trio async/await framework. -* `flake8-trio `__ - Highly opinionated linter for various sorts of problems in Trio and/or AnyIO. Can run as a flake8 plugin, or standalone with support for autofixing some errors. +* `flake8-async `__ - Highly opinionated linter for various sorts of problems in Trio, AnyIO and/or asyncio. Can run as a flake8 plugin, or standalone with support for autofixing some errors. * `tricycle `__ - This is a library of interesting-but-maybe-not-yet-fully-proven extensions to Trio. * `tenacity `__ - Retrying library for Python with async/await support. * `perf-timer `__ - A code timer with Trio async support (see ``TrioPerfTimer``). Collects execution time of a block of code excluding time when the coroutine isn't scheduled, such as during blocking I/O and sleep. Also offers ``trio_perf_counter()`` for low-level timing. From d6e91cc61865329a270cfa2a263924a2a08d1a4b Mon Sep 17 00:00:00 2001 From: jakkdl Date: Thu, 6 Jun 2024 10:55:05 +0200 Subject: [PATCH 02/24] Add explicit documentation on timeout semantic for fail&move_on_after --- src/trio/_timeouts.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/trio/_timeouts.py b/src/trio/_timeouts.py index 1d03b2f2e3..96078d092f 100644 --- a/src/trio/_timeouts.py +++ b/src/trio/_timeouts.py @@ -27,6 +27,9 @@ def move_on_after(seconds: float) -> trio.CancelScope: """Use as a context manager to create a cancel scope whose deadline is set to now + *seconds*. + The deadline of the cancel scope is calculated at creation time, not upon + entering the context manager. + Args: seconds (float): The timeout. @@ -138,6 +141,9 @@ def fail_after(seconds: float) -> AbstractContextManager[trio.CancelScope]: it's caught and discarded. When it reaches :func:`fail_after`, then it's caught and :exc:`TooSlowError` is raised in its place. + The deadline of the cancel scope is calculated at creation time, not upon + entering the context manager. + Args: seconds (float): The timeout. From da2594927ed47003c1f85b863eebdb43f11d5628 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Fri, 7 Jun 2024 12:56:21 +0200 Subject: [PATCH 03/24] use a5rocks alternate phrasing for move_on_after --- src/trio/_timeouts.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/trio/_timeouts.py b/src/trio/_timeouts.py index 96078d092f..deb13147c6 100644 --- a/src/trio/_timeouts.py +++ b/src/trio/_timeouts.py @@ -25,10 +25,7 @@ def move_on_at(deadline: float) -> trio.CancelScope: def move_on_after(seconds: float) -> trio.CancelScope: """Use as a context manager to create a cancel scope whose deadline is - set to now + *seconds*. - - The deadline of the cancel scope is calculated at creation time, not upon - entering the context manager. + set to creation time + *seconds*. Args: seconds (float): The timeout. From 7a8262a806b871abc7c39d4b88ffed79cc37f9a5 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Thu, 20 Jun 2024 22:07:46 +0200 Subject: [PATCH 04/24] add `relative_deadline` attribute to CancelScope. `move_on_after` and `fail_after` now use it, setting the deadline relative to entering the cs, instead of at initialization. --- newsfragments/2512.breaking.rst | 18 +++++++++++++ newsfragments/2512.feature.rst | 13 ++++++++++ src/trio/_core/_run.py | 31 ++++++++++++++++++++++ src/trio/_core/_tests/test_run.py | 43 +++++++++++++++++++++++++++++++ src/trio/_tests/test_timeouts.py | 20 ++++++++++++++ src/trio/_timeouts.py | 28 ++++++++++---------- 6 files changed, 140 insertions(+), 13 deletions(-) create mode 100644 newsfragments/2512.breaking.rst create mode 100644 newsfragments/2512.feature.rst diff --git a/newsfragments/2512.breaking.rst b/newsfragments/2512.breaking.rst new file mode 100644 index 0000000000..22de046fda --- /dev/null +++ b/newsfragments/2512.breaking.rst @@ -0,0 +1,18 @@ +:func:`trio.move_on_after` and :func:`trio.fail_after` now sets the deadline upon entering the context manager, instead of at initialization. The vast majority of usage initialized them when entering and is therefore unaffected. But if you have code that looks like: + +.. code-block:: python3 + + cs = trio._move_on_after(5) + ... + with cs: + ... + + +the deadline of the underlying :func:`trio.CancelScope` will now differ. If you want to fully preserve current behaviour you can rewrite it as + +.. code-block:: python3 + + cs = trio.move_on_at(trio.current_time() + 5) + ... + with cs: + ... diff --git a/newsfragments/2512.feature.rst b/newsfragments/2512.feature.rst new file mode 100644 index 0000000000..141fc35e87 --- /dev/null +++ b/newsfragments/2512.feature.rst @@ -0,0 +1,13 @@ +:func:`trio.CancelScope` now has an attribute ``relative_deadline`` that can be used together with, or instead of, ``deadline``. :ref:`trio.move_on_after` and :ref:`trio.fail_after` now use this functionality in order to resolve the absolute deadline upon entering the context manager. + +If setting both ``deadline`` and ``relative_deadline`` before entering the cm, the deadline will be set to ``min(deadline, trio.current_time() + relative_deadline)``. +Accessing ``relative_deadline`` after entering will return remaining time until deadline (i.e. ``deadline - trio.current_time()``. Setting ``relative_deadline`` after entering will set ``deadline`` to ``trio.current_time() + relative_deadline`` + +Example: + +.. code-block:: python3 + + my_cs = trio.CancelScope(relative_deadline = 5) + ... + with my_cs: # my_cs.deadline will now be 5 seconds into the future + ... diff --git a/src/trio/_core/_run.py b/src/trio/_core/_run.py index 89759cc2c2..2d7765f476 100644 --- a/src/trio/_core/_run.py +++ b/src/trio/_core/_run.py @@ -540,6 +540,11 @@ class CancelScope: # Constructor arguments: _deadline: float = attrs.field(default=inf, kw_only=True, alias="deadline") + # use float|None, or math.inf? + # or take it as an opportunity to also introduce deadline=None? + _relative_deadline: float | None = attrs.field( + default=None, kw_only=True, alias="relative_deadline" + ) _shield: bool = attrs.field(default=False, kw_only=True, alias="shield") @enable_ki_protection @@ -550,6 +555,13 @@ def __enter__(self) -> Self: "Each CancelScope may only be used for a single 'with' block" ) self._has_been_entered = True + + if self._relative_deadline is not None: + if self._relative_deadline + current_time() < self._deadline: + self._deadline = self._relative_deadline + current_time() + else: + self._relative_deadline = self.deadline - current_time() + if current_time() >= self._deadline: self.cancel() with self._might_change_registered_deadline(): @@ -744,6 +756,25 @@ def deadline(self, new_deadline: float) -> None: with self._might_change_registered_deadline(): self._deadline = float(new_deadline) + @property + def relative_deadline(self) -> float | None: + """TODO: write docstring""" + if self._has_been_entered: + if self.deadline == inf: + return None + return self._deadline - current_time() + return self._relative_deadline + + @relative_deadline.setter + def relative_deadline(self, relative_deadline: float | None) -> None: + self._relative_deadline = relative_deadline + if self._has_been_entered: + with self._might_change_registered_deadline(): + if relative_deadline is None: + self._deadline = inf + else: + self._deadline = current_time() + relative_deadline + @property def shield(self) -> bool: """Read-write, :class:`bool`, default :data:`False`. So long as diff --git a/src/trio/_core/_tests/test_run.py b/src/trio/_core/_tests/test_run.py index d54d9f1813..f62abd729f 100644 --- a/src/trio/_core/_tests/test_run.py +++ b/src/trio/_core/_tests/test_run.py @@ -601,6 +601,49 @@ async def test_basic_timeout(mock_clock: _core.MockClock) -> None: await _core.checkpoint() +async def test_relative_timeout(mock_clock: _core.MockClock) -> None: + # test defaults + scope = _core.CancelScope() + assert scope.deadline == inf + assert scope.relative_deadline is None + + # setting relative_deadline before entering does not modify deadline + scope.relative_deadline = 1 + assert scope.deadline == inf + assert scope.relative_deadline == 1 + + # check that setting either deadline updates the other (after entering) + start = _core.current_time() + with scope: + assert scope.deadline == start + 1 + scope.relative_deadline = 2 + assert scope.deadline == start + 2 + scope.deadline = start + 3 + assert scope.relative_deadline == 3 + + # relative_deadline < deadline, deadline is updated + with _core.CancelScope(relative_deadline=1, deadline=start + 2) as scope: + assert scope.deadline == start + 1 + assert scope.relative_deadline == 1 + + # deadline < relative_deadline, relative_deadline is updated + with _core.CancelScope(relative_deadline=2, deadline=start + 1): + assert scope.deadline == start + 1 + assert scope.relative_deadline == 1 + + # once entered, accessing relative_deadline returns deadline-now() + # and setting it calculates it relative to now() + start = _core.current_time() + with _core.CancelScope(relative_deadline=3) as scope: + mock_clock.jump(1) + assert scope.relative_deadline == 2 + mock_clock.jump(1) + assert scope.relative_deadline == 1 + scope.relative_deadline = 3 + mock_clock.jump(1) + assert scope.relative_deadline == 2 + + async def test_cancel_scope_nesting() -> None: # Nested scopes: if two triggering at once, the outer one wins with _core.CancelScope() as scope1: diff --git a/src/trio/_tests/test_timeouts.py b/src/trio/_tests/test_timeouts.py index 98c3d18def..274bcfa29c 100644 --- a/src/trio/_tests/test_timeouts.py +++ b/src/trio/_tests/test_timeouts.py @@ -129,3 +129,23 @@ async def test_timeouts_raise_value_error() -> None: ): with cm(val): pass # pragma: no cover + + +async def test_timeout_deadline_on_entry(mock_clock: _core.MockClock) -> None: + cs = move_on_after(5) + assert cs.relative_deadline == 5 + mock_clock.jump(3) + start = _core.current_time() + with cs: + # This would previously be start+2 + assert cs.deadline == start + 5 + assert cs.relative_deadline == 5 + + # because fail_after is a wrapping contextmanager we cannot directly inspect + # cs_gen.relative_deadline before entering + cs_gen = fail_after(5) + mock_clock.jump(3) + start = _core.current_time() + with cs_gen as cs: + assert cs.deadline == start + 5 + assert cs.relative_deadline == 5 diff --git a/src/trio/_timeouts.py b/src/trio/_timeouts.py index deb13147c6..4d29e2a11c 100644 --- a/src/trio/_timeouts.py +++ b/src/trio/_timeouts.py @@ -1,11 +1,14 @@ from __future__ import annotations import math -from contextlib import AbstractContextManager, contextmanager +from contextlib import contextmanager from typing import TYPE_CHECKING import trio +if TYPE_CHECKING: + from collections.abc import Generator + def move_on_at(deadline: float) -> trio.CancelScope: """Use as a context manager to create a cancel scope with the given @@ -36,7 +39,9 @@ def move_on_after(seconds: float) -> trio.CancelScope: """ if seconds < 0: raise ValueError("timeout must be non-negative") - return move_on_at(trio.current_time() + seconds) + if math.isnan(seconds): + raise ValueError("timeout must not be NaN") + return trio.CancelScope(relative_deadline=seconds) async def sleep_forever() -> None: @@ -94,9 +99,8 @@ class TooSlowError(Exception): """ -# workaround for PyCharm not being able to infer return type from @contextmanager -# see https://youtrack.jetbrains.com/issue/PY-36444/PyCharm-doesnt-infer-types-when-using-contextlib.contextmanager-decorator -def fail_at(deadline: float) -> AbstractContextManager[trio.CancelScope]: # type: ignore[misc] +@contextmanager +def fail_at(deadline: float) -> Generator[trio.CancelScope, None, None]: """Creates a cancel scope with the given deadline, and raises an error if it is actually cancelled. @@ -123,11 +127,8 @@ def fail_at(deadline: float) -> AbstractContextManager[trio.CancelScope]: # typ raise TooSlowError -if not TYPE_CHECKING: - fail_at = contextmanager(fail_at) - - -def fail_after(seconds: float) -> AbstractContextManager[trio.CancelScope]: +@contextmanager +def fail_after(seconds: float) -> Generator[trio.CancelScope, None, None]: """Creates a cancel scope with the given timeout, and raises an error if it is actually cancelled. @@ -150,6 +151,7 @@ def fail_after(seconds: float) -> AbstractContextManager[trio.CancelScope]: ValueError: if *seconds* is less than zero or NaN. """ - if seconds < 0: - raise ValueError("timeout must be non-negative") - return fail_at(trio.current_time() + seconds) + with move_on_after(seconds) as scope: + yield scope + if scope.cancelled_caught: + raise TooSlowError From 5447f456c995b4701a890b4eff2f2378e09bdebd Mon Sep 17 00:00:00 2001 From: jakkdl Date: Thu, 20 Jun 2024 22:30:04 +0200 Subject: [PATCH 05/24] fix codecov, mypy now fails to unify return types across the timeouts in test --- src/trio/_core/_tests/test_run.py | 11 +++++++++++ src/trio/_tests/test_timeouts.py | 7 +++++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/trio/_core/_tests/test_run.py b/src/trio/_core/_tests/test_run.py index f62abd729f..b1415df714 100644 --- a/src/trio/_core/_tests/test_run.py +++ b/src/trio/_core/_tests/test_run.py @@ -643,6 +643,17 @@ async def test_relative_timeout(mock_clock: _core.MockClock) -> None: mock_clock.jump(1) assert scope.relative_deadline == 2 + # no deadline / infinite deadline + with _core.CancelScope() as scope: + assert scope.deadline == inf + assert scope.relative_deadline is None + + start = _core.current_time() + with _core.CancelScope(relative_deadline=1) as scope: + assert scope.deadline == start + 1 + scope.relative_deadline = None + assert scope.deadline == inf + async def test_cancel_scope_nesting() -> None: # Nested scopes: if two triggering at once, the outer one wins diff --git a/src/trio/_tests/test_timeouts.py b/src/trio/_tests/test_timeouts.py index 274bcfa29c..bb3b79e4a7 100644 --- a/src/trio/_tests/test_timeouts.py +++ b/src/trio/_tests/test_timeouts.py @@ -1,5 +1,5 @@ import time -from typing import Awaitable, Callable, TypeVar +from typing import Awaitable, Callable, ContextManager, TypeVar import outcome import pytest @@ -115,7 +115,10 @@ async def test_timeouts_raise_value_error() -> None: ): await fun(val) - for cm, val in ( + cm: Callable[[float], ContextManager[_core.CancelScope]] + # mypy resolves the tuple as containing `Callable[[float], object]`, failing to see + # that both callables are compatible with returning `Contextmanager[CancelScope]` + for cm, val in ( # type: ignore[assignment] (fail_after, -1), (fail_after, nan), (fail_at, nan), From 138156098f8e390d8ecde539bf6088566d200db8 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Mon, 24 Jun 2024 16:13:51 +0200 Subject: [PATCH 06/24] instead of a breaking change we now raise deprecationwarning and give a parameter to opt into new functionality and silence warning. --- newsfragments/2512.breaking.rst | 18 --------- newsfragments/2512.deprecated.rst | 9 +++++ src/trio/_core/_run.py | 42 +++++++++++++++++--- src/trio/_tests/test_timeouts.py | 21 +++++++++- src/trio/_timeouts.py | 65 ++++++++++++++++++++++++++----- 5 files changed, 120 insertions(+), 35 deletions(-) delete mode 100644 newsfragments/2512.breaking.rst create mode 100644 newsfragments/2512.deprecated.rst diff --git a/newsfragments/2512.breaking.rst b/newsfragments/2512.breaking.rst deleted file mode 100644 index 22de046fda..0000000000 --- a/newsfragments/2512.breaking.rst +++ /dev/null @@ -1,18 +0,0 @@ -:func:`trio.move_on_after` and :func:`trio.fail_after` now sets the deadline upon entering the context manager, instead of at initialization. The vast majority of usage initialized them when entering and is therefore unaffected. But if you have code that looks like: - -.. code-block:: python3 - - cs = trio._move_on_after(5) - ... - with cs: - ... - - -the deadline of the underlying :func:`trio.CancelScope` will now differ. If you want to fully preserve current behaviour you can rewrite it as - -.. code-block:: python3 - - cs = trio.move_on_at(trio.current_time() + 5) - ... - with cs: - ... diff --git a/newsfragments/2512.deprecated.rst b/newsfragments/2512.deprecated.rst new file mode 100644 index 0000000000..727e10ff8f --- /dev/null +++ b/newsfragments/2512.deprecated.rst @@ -0,0 +1,9 @@ +:func:`trio.move_on_after` and :func:`trio.fail_after` previously set the deadline relative to initialization time, instead of more intuitively upon entering the context manager. Any code where these two points in time do not line up (according to :func:`trio.current_time`) will now issue a `DeprecationWarning`. To silence the warning and adopt the new behaviour, pass ``timeout_from_enter = True``. If you want to retain current behaviour, you should instead use :func:`trio.move_on_at`/:func:`trio.fail_at`. For example: + +.. code-block:: python3 + + # this is equivalent to how trio.move_on_after(5) would function previously + cs = trio.move_on_at(trio.current_time() + 5) + ... + with cs: + ... diff --git a/src/trio/_core/_run.py b/src/trio/_core/_run.py index f1a85afdd2..5936d4d8f9 100644 --- a/src/trio/_core/_run.py +++ b/src/trio/_core/_run.py @@ -550,6 +550,13 @@ class CancelScope: ) _shield: bool = attrs.field(default=False, kw_only=True, alias="shield") + # introduced during deprecation period of handling relative timeouts from + # initialization + _timeout_from_enter: bool = attrs.field( + default=True, kw_only=True, alias="_timeout_from_enter" + ) + _creation_time: float | None = attrs.field(default=None, alias="_creation_time") + @enable_ki_protection def __enter__(self) -> Self: task = _core.current_task() @@ -557,13 +564,38 @@ def __enter__(self) -> Self: raise RuntimeError( "Each CancelScope may only be used for a single 'with' block" ) + + if ( + self._creation_time is not None + and abs(self._creation_time - current_time()) > 0.01 + and not self._timeout_from_enter + ): + # not using warn_deprecated because the message template is a weird fit + # TODO: mention versions in the message? + warnings.warn( + DeprecationWarning( + "`move_on_after` and `fail_after` will change behaviour to " + "start the deadline relative to entering the cm, instead of " + "at creation time. To silence this warning and opt into the " + "new behaviour, pass `timeout_from_enter=True`. " + "To keep old behaviour, use `move_on_at(trio.current_time() + x)` " + "(or `fail_at`), where `x` is the previous timeout length. " + "See https://github.com/python-trio/trio/issues/2512" + ), + stacklevel=2, + ) + assert ( + self._relative_deadline is not None + ), "User is manually passing `_timeout_from_enter=False`. Don't do that." + self._relative_deadline -= current_time() - self._creation_time + self._has_been_entered = True - if self._relative_deadline is not None: - if self._relative_deadline + current_time() < self._deadline: - self._deadline = self._relative_deadline + current_time() - else: - self._relative_deadline = self.deadline - current_time() + if ( + self._relative_deadline is not None + and self._relative_deadline + current_time() < self._deadline + ): + self._deadline = self._relative_deadline + current_time() if current_time() >= self._deadline: self.cancel() diff --git a/src/trio/_tests/test_timeouts.py b/src/trio/_tests/test_timeouts.py index bb3b79e4a7..ebda5393e3 100644 --- a/src/trio/_tests/test_timeouts.py +++ b/src/trio/_tests/test_timeouts.py @@ -135,7 +135,7 @@ async def test_timeouts_raise_value_error() -> None: async def test_timeout_deadline_on_entry(mock_clock: _core.MockClock) -> None: - cs = move_on_after(5) + cs = move_on_after(5, timeout_from_enter=True) assert cs.relative_deadline == 5 mock_clock.jump(3) start = _core.current_time() @@ -146,9 +146,26 @@ async def test_timeout_deadline_on_entry(mock_clock: _core.MockClock) -> None: # because fail_after is a wrapping contextmanager we cannot directly inspect # cs_gen.relative_deadline before entering - cs_gen = fail_after(5) + cs_gen = fail_after(5, timeout_from_enter=True) mock_clock.jump(3) start = _core.current_time() with cs_gen as cs: assert cs.deadline == start + 5 assert cs.relative_deadline == 5 + + +async def test_timeout_deadline_not_on_entry(mock_clock: _core.MockClock) -> None: + """Test that not setting timeout_from_enter gives a DeprecationWarning and + retains old behaviour.""" + with pytest.warns(DeprecationWarning, match="issues/2512"): + cs = move_on_after(5) + mock_clock.jump(3) + with cs: + assert cs.deadline - _core.current_time() == 2 + assert cs.relative_deadline == 2 + with pytest.warns(DeprecationWarning, match="issues/2512"): + cs_gen = fail_after(5) + mock_clock.jump(3) + with cs_gen as cs: + assert cs.deadline - _core.current_time() == 2 + assert cs.relative_deadline == 2 diff --git a/src/trio/_timeouts.py b/src/trio/_timeouts.py index 4d29e2a11c..7f01e2ae3d 100644 --- a/src/trio/_timeouts.py +++ b/src/trio/_timeouts.py @@ -1,13 +1,18 @@ from __future__ import annotations import math -from contextlib import contextmanager -from typing import TYPE_CHECKING +from contextlib import AbstractContextManager, contextmanager +from typing import TYPE_CHECKING, TypeVar import trio +from ._util import final + +T = TypeVar("T") + if TYPE_CHECKING: from collections.abc import Generator + from types import TracebackType def move_on_at(deadline: float) -> trio.CancelScope: @@ -26,7 +31,12 @@ def move_on_at(deadline: float) -> trio.CancelScope: return trio.CancelScope(deadline=deadline) -def move_on_after(seconds: float) -> trio.CancelScope: +def move_on_after( + seconds: float, + *, + timeout_from_enter: bool = False, + _creation_time: float | None = None, +) -> trio.CancelScope: """Use as a context manager to create a cancel scope whose deadline is set to creation time + *seconds*. @@ -41,7 +51,13 @@ def move_on_after(seconds: float) -> trio.CancelScope: raise ValueError("timeout must be non-negative") if math.isnan(seconds): raise ValueError("timeout must not be NaN") - return trio.CancelScope(relative_deadline=seconds) + if _creation_time is None: + _creation_time = trio.current_time() + return trio.CancelScope( + relative_deadline=seconds, + _timeout_from_enter=timeout_from_enter, + _creation_time=_creation_time, + ) async def sleep_forever() -> None: @@ -127,8 +143,14 @@ def fail_at(deadline: float) -> Generator[trio.CancelScope, None, None]: raise TooSlowError -@contextmanager -def fail_after(seconds: float) -> Generator[trio.CancelScope, None, None]: +# This was previously a simple @contextmanager-based cm, but in order to save +# the current time at initialization it needed to be class-based. +# Once that change of behaviour has gone through and the old functionality is removed +# it can be reverted. + + +@final +class fail_after(AbstractContextManager[trio.CancelScope]): """Creates a cancel scope with the given timeout, and raises an error if it is actually cancelled. @@ -151,7 +173,30 @@ def fail_after(seconds: float) -> Generator[trio.CancelScope, None, None]: ValueError: if *seconds* is less than zero or NaN. """ - with move_on_after(seconds) as scope: - yield scope - if scope.cancelled_caught: - raise TooSlowError + + def __init__(self, seconds: float, *, timeout_from_enter: bool = False): + self.seconds = seconds + self.timeout_from_enter = timeout_from_enter + self._creation_time = trio.current_time() + self.scope: trio.CancelScope | None = None + + def __enter__(self) -> trio.CancelScope: + self.scope = trio.move_on_after( + self.seconds, + timeout_from_enter=self.timeout_from_enter, + _creation_time=self._creation_time, + ) + return self.scope.__enter__() + + def __exit__( + self, + exc_type: type[BaseException] | None, + exc_value: BaseException | None, + traceback: TracebackType | None, + ) -> bool | None: + if self.scope is None: + raise RuntimeError("__exit__ called before __enter__") + result = self.scope.__exit__(exc_type, exc_value, traceback) + if self.scope.cancelled_caught: + raise TooSlowError + return result From 742c013d4771be69160bea9ded3366762033c1cd Mon Sep 17 00:00:00 2001 From: jakkdl Date: Mon, 24 Jun 2024 16:25:21 +0200 Subject: [PATCH 07/24] don't inherit from AbstractContextManager since that breaks 3.8, and use private names for attributes --- src/trio/_timeouts.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/trio/_timeouts.py b/src/trio/_timeouts.py index 7f01e2ae3d..5e8add3d2b 100644 --- a/src/trio/_timeouts.py +++ b/src/trio/_timeouts.py @@ -1,7 +1,7 @@ from __future__ import annotations import math -from contextlib import AbstractContextManager, contextmanager +from contextlib import contextmanager from typing import TYPE_CHECKING, TypeVar import trio @@ -150,7 +150,7 @@ def fail_at(deadline: float) -> Generator[trio.CancelScope, None, None]: @final -class fail_after(AbstractContextManager[trio.CancelScope]): +class fail_after: """Creates a cancel scope with the given timeout, and raises an error if it is actually cancelled. @@ -175,18 +175,18 @@ class fail_after(AbstractContextManager[trio.CancelScope]): """ def __init__(self, seconds: float, *, timeout_from_enter: bool = False): - self.seconds = seconds - self.timeout_from_enter = timeout_from_enter + self._seconds = seconds + self._timeout_from_enter = timeout_from_enter self._creation_time = trio.current_time() - self.scope: trio.CancelScope | None = None + self._scope: trio.CancelScope | None = None def __enter__(self) -> trio.CancelScope: - self.scope = trio.move_on_after( - self.seconds, - timeout_from_enter=self.timeout_from_enter, + self._scope = trio.move_on_after( + self._seconds, + timeout_from_enter=self._timeout_from_enter, _creation_time=self._creation_time, ) - return self.scope.__enter__() + return self._scope.__enter__() def __exit__( self, @@ -194,9 +194,9 @@ def __exit__( exc_value: BaseException | None, traceback: TracebackType | None, ) -> bool | None: - if self.scope is None: + if self._scope is None: raise RuntimeError("__exit__ called before __enter__") - result = self.scope.__exit__(exc_type, exc_value, traceback) - if self.scope.cancelled_caught: + result = self._scope.__exit__(exc_type, exc_value, traceback) + if self._scope.cancelled_caught: raise TooSlowError return result From 0075b56185ee6e16e1a326daf6729e1a785422d6 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Mon, 24 Jun 2024 17:13:32 +0200 Subject: [PATCH 08/24] fix RTD build. Replace star import with explicit list. Remove useless TypeVar. --- newsfragments/2512.deprecated.rst | 2 +- newsfragments/2512.feature.rst | 4 ++-- src/trio/_tests/test_timeouts.py | 10 +++++++++- src/trio/_timeouts.py | 6 ++---- 4 files changed, 14 insertions(+), 8 deletions(-) diff --git a/newsfragments/2512.deprecated.rst b/newsfragments/2512.deprecated.rst index 727e10ff8f..1315386aa2 100644 --- a/newsfragments/2512.deprecated.rst +++ b/newsfragments/2512.deprecated.rst @@ -1,6 +1,6 @@ :func:`trio.move_on_after` and :func:`trio.fail_after` previously set the deadline relative to initialization time, instead of more intuitively upon entering the context manager. Any code where these two points in time do not line up (according to :func:`trio.current_time`) will now issue a `DeprecationWarning`. To silence the warning and adopt the new behaviour, pass ``timeout_from_enter = True``. If you want to retain current behaviour, you should instead use :func:`trio.move_on_at`/:func:`trio.fail_at`. For example: -.. code-block:: python3 +.. code-block:: python # this is equivalent to how trio.move_on_after(5) would function previously cs = trio.move_on_at(trio.current_time() + 5) diff --git a/newsfragments/2512.feature.rst b/newsfragments/2512.feature.rst index 141fc35e87..1c9e2222e6 100644 --- a/newsfragments/2512.feature.rst +++ b/newsfragments/2512.feature.rst @@ -1,11 +1,11 @@ -:func:`trio.CancelScope` now has an attribute ``relative_deadline`` that can be used together with, or instead of, ``deadline``. :ref:`trio.move_on_after` and :ref:`trio.fail_after` now use this functionality in order to resolve the absolute deadline upon entering the context manager. +:func:`trio.CancelScope` now has an attribute ``relative_deadline`` that can be used together with, or instead of, ``deadline``. :func:`trio.move_on_after` and :func:`trio.fail_after` now use this functionality in order to resolve the absolute deadline upon entering the context manager. If setting both ``deadline`` and ``relative_deadline`` before entering the cm, the deadline will be set to ``min(deadline, trio.current_time() + relative_deadline)``. Accessing ``relative_deadline`` after entering will return remaining time until deadline (i.e. ``deadline - trio.current_time()``. Setting ``relative_deadline`` after entering will set ``deadline`` to ``trio.current_time() + relative_deadline`` Example: -.. code-block:: python3 +.. code-block:: python my_cs = trio.CancelScope(relative_deadline = 5) ... diff --git a/src/trio/_tests/test_timeouts.py b/src/trio/_tests/test_timeouts.py index ebda5393e3..2553fcff5a 100644 --- a/src/trio/_tests/test_timeouts.py +++ b/src/trio/_tests/test_timeouts.py @@ -6,7 +6,15 @@ from .. import _core from .._core._tests.tutil import slow -from .._timeouts import * +from .._timeouts import ( + TooSlowError, + fail_after, + fail_at, + move_on_after, + move_on_at, + sleep, + sleep_until, +) from ..testing import assert_checkpoints T = TypeVar("T") diff --git a/src/trio/_timeouts.py b/src/trio/_timeouts.py index 5e8add3d2b..55634f245c 100644 --- a/src/trio/_timeouts.py +++ b/src/trio/_timeouts.py @@ -2,14 +2,12 @@ import math from contextlib import contextmanager -from typing import TYPE_CHECKING, TypeVar +from typing import TYPE_CHECKING import trio from ._util import final -T = TypeVar("T") - if TYPE_CHECKING: from collections.abc import Generator from types import TracebackType @@ -194,7 +192,7 @@ def __exit__( exc_value: BaseException | None, traceback: TracebackType | None, ) -> bool | None: - if self._scope is None: + if self._scope is None: # pragma: no cover raise RuntimeError("__exit__ called before __enter__") result = self._scope.__exit__(exc_type, exc_value, traceback) if self._scope.cancelled_caught: From 9d3d0b956e8e2d90e8b0d501f096718d27e715c2 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Wed, 26 Jun 2024 15:43:21 +0200 Subject: [PATCH 09/24] reimplement as transforming class --- src/trio/_core/_run.py | 63 ---------------- src/trio/_core/_tests/test_run.py | 54 -------------- src/trio/_tests/test_timeouts.py | 21 ++---- src/trio/_timeouts.py | 117 +++++++++++++++++++----------- 4 files changed, 81 insertions(+), 174 deletions(-) diff --git a/src/trio/_core/_run.py b/src/trio/_core/_run.py index 5936d4d8f9..1512fdf954 100644 --- a/src/trio/_core/_run.py +++ b/src/trio/_core/_run.py @@ -543,20 +543,8 @@ class CancelScope: # Constructor arguments: _deadline: float = attrs.field(default=inf, kw_only=True, alias="deadline") - # use float|None, or math.inf? - # or take it as an opportunity to also introduce deadline=None? - _relative_deadline: float | None = attrs.field( - default=None, kw_only=True, alias="relative_deadline" - ) _shield: bool = attrs.field(default=False, kw_only=True, alias="shield") - # introduced during deprecation period of handling relative timeouts from - # initialization - _timeout_from_enter: bool = attrs.field( - default=True, kw_only=True, alias="_timeout_from_enter" - ) - _creation_time: float | None = attrs.field(default=None, alias="_creation_time") - @enable_ki_protection def __enter__(self) -> Self: task = _core.current_task() @@ -564,39 +552,7 @@ def __enter__(self) -> Self: raise RuntimeError( "Each CancelScope may only be used for a single 'with' block" ) - - if ( - self._creation_time is not None - and abs(self._creation_time - current_time()) > 0.01 - and not self._timeout_from_enter - ): - # not using warn_deprecated because the message template is a weird fit - # TODO: mention versions in the message? - warnings.warn( - DeprecationWarning( - "`move_on_after` and `fail_after` will change behaviour to " - "start the deadline relative to entering the cm, instead of " - "at creation time. To silence this warning and opt into the " - "new behaviour, pass `timeout_from_enter=True`. " - "To keep old behaviour, use `move_on_at(trio.current_time() + x)` " - "(or `fail_at`), where `x` is the previous timeout length. " - "See https://github.com/python-trio/trio/issues/2512" - ), - stacklevel=2, - ) - assert ( - self._relative_deadline is not None - ), "User is manually passing `_timeout_from_enter=False`. Don't do that." - self._relative_deadline -= current_time() - self._creation_time - self._has_been_entered = True - - if ( - self._relative_deadline is not None - and self._relative_deadline + current_time() < self._deadline - ): - self._deadline = self._relative_deadline + current_time() - if current_time() >= self._deadline: self.cancel() with self._might_change_registered_deadline(): @@ -784,25 +740,6 @@ def deadline(self, new_deadline: float) -> None: with self._might_change_registered_deadline(): self._deadline = float(new_deadline) - @property - def relative_deadline(self) -> float | None: - """TODO: write docstring""" - if self._has_been_entered: - if self.deadline == inf: - return None - return self._deadline - current_time() - return self._relative_deadline - - @relative_deadline.setter - def relative_deadline(self, relative_deadline: float | None) -> None: - self._relative_deadline = relative_deadline - if self._has_been_entered: - with self._might_change_registered_deadline(): - if relative_deadline is None: - self._deadline = inf - else: - self._deadline = current_time() + relative_deadline - @property def shield(self) -> bool: """Read-write, :class:`bool`, default :data:`False`. So long as diff --git a/src/trio/_core/_tests/test_run.py b/src/trio/_core/_tests/test_run.py index b1415df714..d54d9f1813 100644 --- a/src/trio/_core/_tests/test_run.py +++ b/src/trio/_core/_tests/test_run.py @@ -601,60 +601,6 @@ async def test_basic_timeout(mock_clock: _core.MockClock) -> None: await _core.checkpoint() -async def test_relative_timeout(mock_clock: _core.MockClock) -> None: - # test defaults - scope = _core.CancelScope() - assert scope.deadline == inf - assert scope.relative_deadline is None - - # setting relative_deadline before entering does not modify deadline - scope.relative_deadline = 1 - assert scope.deadline == inf - assert scope.relative_deadline == 1 - - # check that setting either deadline updates the other (after entering) - start = _core.current_time() - with scope: - assert scope.deadline == start + 1 - scope.relative_deadline = 2 - assert scope.deadline == start + 2 - scope.deadline = start + 3 - assert scope.relative_deadline == 3 - - # relative_deadline < deadline, deadline is updated - with _core.CancelScope(relative_deadline=1, deadline=start + 2) as scope: - assert scope.deadline == start + 1 - assert scope.relative_deadline == 1 - - # deadline < relative_deadline, relative_deadline is updated - with _core.CancelScope(relative_deadline=2, deadline=start + 1): - assert scope.deadline == start + 1 - assert scope.relative_deadline == 1 - - # once entered, accessing relative_deadline returns deadline-now() - # and setting it calculates it relative to now() - start = _core.current_time() - with _core.CancelScope(relative_deadline=3) as scope: - mock_clock.jump(1) - assert scope.relative_deadline == 2 - mock_clock.jump(1) - assert scope.relative_deadline == 1 - scope.relative_deadline = 3 - mock_clock.jump(1) - assert scope.relative_deadline == 2 - - # no deadline / infinite deadline - with _core.CancelScope() as scope: - assert scope.deadline == inf - assert scope.relative_deadline is None - - start = _core.current_time() - with _core.CancelScope(relative_deadline=1) as scope: - assert scope.deadline == start + 1 - scope.relative_deadline = None - assert scope.deadline == inf - - async def test_cancel_scope_nesting() -> None: # Nested scopes: if two triggering at once, the outer one wins with _core.CancelScope() as scope1: diff --git a/src/trio/_tests/test_timeouts.py b/src/trio/_tests/test_timeouts.py index 2553fcff5a..552b07fa00 100644 --- a/src/trio/_tests/test_timeouts.py +++ b/src/trio/_tests/test_timeouts.py @@ -143,37 +143,32 @@ async def test_timeouts_raise_value_error() -> None: async def test_timeout_deadline_on_entry(mock_clock: _core.MockClock) -> None: - cs = move_on_after(5, timeout_from_enter=True) - assert cs.relative_deadline == 5 + rcs = move_on_after(5, timeout_from_enter=True) + assert rcs.relative_deadline == 5 mock_clock.jump(3) start = _core.current_time() - with cs: + with rcs as cs: # This would previously be start+2 assert cs.deadline == start + 5 - assert cs.relative_deadline == 5 - # because fail_after is a wrapping contextmanager we cannot directly inspect - # cs_gen.relative_deadline before entering - cs_gen = fail_after(5, timeout_from_enter=True) + rcs = fail_after(5, timeout_from_enter=True) mock_clock.jump(3) start = _core.current_time() - with cs_gen as cs: + with rcs as cs: assert cs.deadline == start + 5 - assert cs.relative_deadline == 5 async def test_timeout_deadline_not_on_entry(mock_clock: _core.MockClock) -> None: """Test that not setting timeout_from_enter gives a DeprecationWarning and retains old behaviour.""" with pytest.warns(DeprecationWarning, match="issues/2512"): - cs = move_on_after(5) + rcs = move_on_after(5) mock_clock.jump(3) - with cs: + with rcs as cs: assert cs.deadline - _core.current_time() == 2 - assert cs.relative_deadline == 2 + with pytest.warns(DeprecationWarning, match="issues/2512"): cs_gen = fail_after(5) mock_clock.jump(3) with cs_gen as cs: assert cs.deadline - _core.current_time() == 2 - assert cs.relative_deadline == 2 diff --git a/src/trio/_timeouts.py b/src/trio/_timeouts.py index 55634f245c..ba9a6d621a 100644 --- a/src/trio/_timeouts.py +++ b/src/trio/_timeouts.py @@ -1,6 +1,7 @@ from __future__ import annotations import math +import warnings from contextlib import contextmanager from typing import TYPE_CHECKING @@ -13,6 +14,63 @@ from types import TracebackType +@final +class _RelativeCancelScope: + """Makes it possible to specify relative deadlines at initialization, that does + not start counting until the cm is entered. + Upon entering it returns a CancelScope, so unless initialization and entering + are separate, this class will be transparent to end users. + """ + + def __init__(self, relative_deadline: float, *, timeout_from_enter: bool = False): + self.relative_deadline = relative_deadline + self._timeout_from_enter = timeout_from_enter + + self._fail: bool = False + self._creation_time = trio.current_time() + self._scope: trio.CancelScope | None = None + + def __enter__(self) -> trio.CancelScope: + if ( + abs(self._creation_time - trio.current_time()) > 0.01 + and not self._timeout_from_enter + ): + # not using warn_deprecated because the message template is a weird fit + # TODO: mention versions in the message? + warnings.warn( + DeprecationWarning( + "`move_on_after` and `fail_after` will change behaviour to " + "start the deadline relative to entering the cm, instead of " + "at creation time. To silence this warning and opt into the " + "new behaviour, pass `timeout_from_enter=True`. " + "To keep old behaviour, use `move_on_at(trio.current_time() + x)` " + "(or `fail_at`), where `x` is the previous timeout length. " + "See https://github.com/python-trio/trio/issues/2512" + ), + stacklevel=2, + ) + + if self._timeout_from_enter: + start_time = trio.current_time() + else: + start_time = self._creation_time + + self._scope = trio.CancelScope(deadline=start_time + self.relative_deadline) + return self._scope + + def __exit__( + self, + exc_type: type[BaseException] | None, + exc_value: BaseException | None, + traceback: TracebackType | None, + ) -> bool | None: + if self._scope is None: # pragma: no cover + raise RuntimeError("__exit__ called before __enter__") + if self._fail and self._scope.cancelled_caught: + raise TooSlowError + return None + + def move_on_at(deadline: float) -> trio.CancelScope: """Use as a context manager to create a cancel scope with the given absolute deadline. @@ -33,8 +91,7 @@ def move_on_after( seconds: float, *, timeout_from_enter: bool = False, - _creation_time: float | None = None, -) -> trio.CancelScope: +) -> _RelativeCancelScope: """Use as a context manager to create a cancel scope whose deadline is set to creation time + *seconds*. @@ -49,12 +106,9 @@ def move_on_after( raise ValueError("timeout must be non-negative") if math.isnan(seconds): raise ValueError("timeout must not be NaN") - if _creation_time is None: - _creation_time = trio.current_time() - return trio.CancelScope( + return _RelativeCancelScope( relative_deadline=seconds, - _timeout_from_enter=timeout_from_enter, - _creation_time=_creation_time, + timeout_from_enter=timeout_from_enter, ) @@ -141,14 +195,11 @@ def fail_at(deadline: float) -> Generator[trio.CancelScope, None, None]: raise TooSlowError -# This was previously a simple @contextmanager-based cm, but in order to save -# the current time at initialization it needed to be class-based. -# Once that change of behaviour has gone through and the old functionality is removed -# it can be reverted. - - -@final -class fail_after: +def fail_after( + seconds: float, + *, + timeout_from_enter: bool = False, +) -> _RelativeCancelScope: """Creates a cancel scope with the given timeout, and raises an error if it is actually cancelled. @@ -159,8 +210,10 @@ class fail_after: it's caught and discarded. When it reaches :func:`fail_after`, then it's caught and :exc:`TooSlowError` is raised in its place. - The deadline of the cancel scope is calculated at creation time, not upon - entering the context manager. + The deadline of the cancel scope was previously calculated at creation time, + not upon entering the context manager. This is still the default, but deprecated. + If you pass ``timeout_from_enter=True`` it will instead be calculated relative + to entering the cm, and silence the deprecationwarning. Args: seconds (float): The timeout. @@ -171,30 +224,6 @@ class fail_after: ValueError: if *seconds* is less than zero or NaN. """ - - def __init__(self, seconds: float, *, timeout_from_enter: bool = False): - self._seconds = seconds - self._timeout_from_enter = timeout_from_enter - self._creation_time = trio.current_time() - self._scope: trio.CancelScope | None = None - - def __enter__(self) -> trio.CancelScope: - self._scope = trio.move_on_after( - self._seconds, - timeout_from_enter=self._timeout_from_enter, - _creation_time=self._creation_time, - ) - return self._scope.__enter__() - - def __exit__( - self, - exc_type: type[BaseException] | None, - exc_value: BaseException | None, - traceback: TracebackType | None, - ) -> bool | None: - if self._scope is None: # pragma: no cover - raise RuntimeError("__exit__ called before __enter__") - result = self._scope.__exit__(exc_type, exc_value, traceback) - if self._scope.cancelled_caught: - raise TooSlowError - return result + rcs = move_on_after(seconds, timeout_from_enter=timeout_from_enter) + rcs._fail = True + return rcs From 480681d1fb7e0502bbb0662d9278a4fb54a9c211 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Thu, 27 Jun 2024 15:27:20 +0200 Subject: [PATCH 10/24] whoops, forgot to actually enter the cancelscope --- src/trio/_tests/test_timeouts.py | 5 +++++ src/trio/_timeouts.py | 4 +++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/trio/_tests/test_timeouts.py b/src/trio/_tests/test_timeouts.py index 552b07fa00..0642e16fbc 100644 --- a/src/trio/_tests/test_timeouts.py +++ b/src/trio/_tests/test_timeouts.py @@ -157,6 +157,11 @@ async def test_timeout_deadline_on_entry(mock_clock: _core.MockClock) -> None: with rcs as cs: assert cs.deadline == start + 5 + # TODO: not implemented + # check that code that does not "re-save" the cancelscope still functions + # assert rcs.deadline == start + 5 + # assert rcs.shield == False + async def test_timeout_deadline_not_on_entry(mock_clock: _core.MockClock) -> None: """Test that not setting timeout_from_enter gives a DeprecationWarning and diff --git a/src/trio/_timeouts.py b/src/trio/_timeouts.py index ba9a6d621a..29dee9c1df 100644 --- a/src/trio/_timeouts.py +++ b/src/trio/_timeouts.py @@ -56,6 +56,7 @@ def __enter__(self) -> trio.CancelScope: start_time = self._creation_time self._scope = trio.CancelScope(deadline=start_time + self.relative_deadline) + self._scope.__enter__() return self._scope def __exit__( @@ -66,9 +67,10 @@ def __exit__( ) -> bool | None: if self._scope is None: # pragma: no cover raise RuntimeError("__exit__ called before __enter__") + res = self._scope.__exit__(exc_type, exc_value, traceback) if self._fail and self._scope.cancelled_caught: raise TooSlowError - return None + return res def move_on_at(deadline: float) -> trio.CancelScope: From 510aaa1685588371cacb453728a4b27d7186d674 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Sat, 31 Aug 2024 13:52:29 +0200 Subject: [PATCH 11/24] remove unneeded type:ignore with mypy 1.11 --- src/trio/_tests/test_timeouts.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/trio/_tests/test_timeouts.py b/src/trio/_tests/test_timeouts.py index b47859857b..83bcd6dd3e 100644 --- a/src/trio/_tests/test_timeouts.py +++ b/src/trio/_tests/test_timeouts.py @@ -1,5 +1,5 @@ import time -from typing import Awaitable, Callable, ContextManager, Protocol, TypeVar +from typing import Awaitable, Callable, Protocol, TypeVar import outcome import pytest @@ -170,10 +170,7 @@ async def test_timeouts_raise_value_error() -> None: ): await fun(val) - cm: Callable[[float], ContextManager[_core.CancelScope]] - # mypy resolves the tuple as containing `Callable[[float], object]`, failing to see - # that both callables are compatible with returning `Contextmanager[CancelScope]` - for cm, val in ( # type: ignore[assignment] + for cm, val in ( (fail_after, -1), (fail_after, nan), (fail_at, nan), From 347412862d14b2f9fc3ca582b8e24b89d3818283 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Thu, 5 Sep 2024 11:28:31 +0200 Subject: [PATCH 12/24] write docs, update newsfragments to match current implementation --- docs/source/reference-core.rst | 37 ++++++++++++++++++++++++++++++- newsfragments/2512.deprecated.rst | 10 +-------- newsfragments/2512.feature.rst | 14 +----------- src/trio/_timeouts.py | 12 ++++++++-- 4 files changed, 48 insertions(+), 25 deletions(-) diff --git a/docs/source/reference-core.rst b/docs/source/reference-core.rst index 392eac7d20..99e392ecb2 100644 --- a/docs/source/reference-core.rst +++ b/docs/source/reference-core.rst @@ -561,7 +561,8 @@ situation of just wanting to impose a timeout on some code: .. autofunction:: fail_at :with: cancel_scope -Cheat sheet: +Cheat sheet ++++++++++++ * If you want to impose a timeout on a function, but you don't care whether it timed out or not: @@ -598,6 +599,40 @@ which is sometimes useful: .. autofunction:: current_effective_deadline +.. _saved_relative_timeouts: + +Separating initialization and entry of a :class:`CancelScope` ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ +Before v0.26.3 there was no separation of initialization and entry of a :class:`CancelScope`. This meant that trying to initialize :func:`move_on_after` or :func:`fail_after` and entering them at a later point meant that the timeout was relative to initialization, leading to confusing behaviour. + +.. code-block:: python + + # THIS WILL NOW GIVE A DEPRECATION ERROR + my_cs = trio.move_on_after(5) + trio.sleep(1) + with my_cs: # this would move on after 4 seconds + ... + +This is now resolved with the addition of a wrapper class :class:`trio._timeouts._RelativeCancelScope` that is transparent to end users when initializing and entering at the same time. To silence the :class:`DeprecationWarning` and use the new behavior, pass ``timeout_from_enter=True`` to :func:`move_on_after` or :func:`fail_after`. + +.. code-block:: python + + my_cs = trio.move_on_after(5, timeout_from_enter = True) + trio.sleep(1) + with my_cs: # this will now move on after 5 seconds + ... + +If you want to retain previous behaviour, you should now instead write something like + +.. code-block:: python + + my_cs = trio.move_on_at(trio.current_time() + 5) + trio.sleep(1) + with my_cs: # this will now unambiguously move on after 4 seconds + ... + +.. autoclass:: trio._timeouts._RelativeCancelScope + .. _tasks: Tasks let you do multiple things at once diff --git a/newsfragments/2512.deprecated.rst b/newsfragments/2512.deprecated.rst index 1315386aa2..bbb098b1c8 100644 --- a/newsfragments/2512.deprecated.rst +++ b/newsfragments/2512.deprecated.rst @@ -1,9 +1 @@ -:func:`trio.move_on_after` and :func:`trio.fail_after` previously set the deadline relative to initialization time, instead of more intuitively upon entering the context manager. Any code where these two points in time do not line up (according to :func:`trio.current_time`) will now issue a `DeprecationWarning`. To silence the warning and adopt the new behaviour, pass ``timeout_from_enter = True``. If you want to retain current behaviour, you should instead use :func:`trio.move_on_at`/:func:`trio.fail_at`. For example: - -.. code-block:: python - - # this is equivalent to how trio.move_on_after(5) would function previously - cs = trio.move_on_at(trio.current_time() + 5) - ... - with cs: - ... +:func:`trio.move_on_after` and :func:`trio.fail_after` previously set the deadline relative to initialization time, instead of more intuitively upon entering the context manager. Any code where these two points in time do not line up (according to :func:`trio.current_time`) will now issue a `DeprecationWarning`. To silence the warning and adopt the new behaviour, pass ``timeout_from_enter = True``. For more information see :ref:`saved_relative_timeouts`. diff --git a/newsfragments/2512.feature.rst b/newsfragments/2512.feature.rst index 1c9e2222e6..d30c891961 100644 --- a/newsfragments/2512.feature.rst +++ b/newsfragments/2512.feature.rst @@ -1,13 +1 @@ -:func:`trio.CancelScope` now has an attribute ``relative_deadline`` that can be used together with, or instead of, ``deadline``. :func:`trio.move_on_after` and :func:`trio.fail_after` now use this functionality in order to resolve the absolute deadline upon entering the context manager. - -If setting both ``deadline`` and ``relative_deadline`` before entering the cm, the deadline will be set to ``min(deadline, trio.current_time() + relative_deadline)``. -Accessing ``relative_deadline`` after entering will return remaining time until deadline (i.e. ``deadline - trio.current_time()``. Setting ``relative_deadline`` after entering will set ``deadline`` to ``trio.current_time() + relative_deadline`` - -Example: - -.. code-block:: python - - my_cs = trio.CancelScope(relative_deadline = 5) - ... - with my_cs: # my_cs.deadline will now be 5 seconds into the future - ... +:func:`trio.move_on_after` and :func:`trio.fail_after` now allows setting the deadline to be relative to entering the context manager. This will be the default in the future, and to opt into the new behaviour you can pass ``timeout_from_enter=True``. This does not matter in the vast majority of cases where initialization and entering of the context manager is in the same place, and those cases will not raise any :class:`DeprecationWarning`. For more information see :ref:`saved_relative_timeouts`. diff --git a/src/trio/_timeouts.py b/src/trio/_timeouts.py index 72e098fd75..8203520f89 100644 --- a/src/trio/_timeouts.py +++ b/src/trio/_timeouts.py @@ -108,7 +108,15 @@ def move_on_after( timeout_from_enter: bool = False, ) -> _RelativeCancelScope: """Use as a context manager to create a cancel scope whose deadline is - set to creation time + *seconds*. + set to now + *seconds*. + + The deadline of the cancel scope was previously calculated at creation time, + not upon entering the context manager. This is still the default, but deprecated. + If you pass ``timeout_from_enter=True`` it will instead be calculated relative + to entering the cm, and silence the :class:`DeprecationWarning`. + + If you're entering the cancel scope at initialization time, which is the most common + use case, you can treat this function as returning a :class:`CancelScope`. Args: seconds (float): The timeout. @@ -238,7 +246,7 @@ def fail_after( The deadline of the cancel scope was previously calculated at creation time, not upon entering the context manager. This is still the default, but deprecated. If you pass ``timeout_from_enter=True`` it will instead be calculated relative - to entering the cm, and silence the deprecationwarning. + to entering the cm, and silence the :class:`DeprecationWarning`. Args: seconds (float): The timeout. From df5876d29acc85b3a27f47d48a841852c5282dd0 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Thu, 5 Sep 2024 13:41:55 +0200 Subject: [PATCH 13/24] add transitional functions --- src/trio/_tests/test_timeouts.py | 97 +++++++++++++++++++++---- src/trio/_timeouts.py | 117 ++++++++++++++++++++++++++++++- 2 files changed, 196 insertions(+), 18 deletions(-) diff --git a/src/trio/_tests/test_timeouts.py b/src/trio/_tests/test_timeouts.py index 83bcd6dd3e..6833726b02 100644 --- a/src/trio/_tests/test_timeouts.py +++ b/src/trio/_tests/test_timeouts.py @@ -189,6 +189,7 @@ async def test_timeouts_raise_value_error() -> None: async def test_timeout_deadline_on_entry(mock_clock: _core.MockClock) -> None: rcs = move_on_after(5, timeout_from_enter=True) assert rcs.relative_deadline == 5 + mock_clock.jump(3) start = _core.current_time() with rcs as cs: @@ -201,23 +202,89 @@ async def test_timeout_deadline_on_entry(mock_clock: _core.MockClock) -> None: with rcs as cs: assert cs.deadline == start + 5 - # TODO: not implemented - # check that code that does not "re-save" the cancelscope still functions - # assert rcs.deadline == start + 5 - # assert rcs.shield == False + # check that their shield values are linked + assert rcs.shield is False + + cs.shield = True + assert rcs.shield is True + + rcs.shield = False + assert cs.shield is False + + # re-entering a _RelativeCancelScope should probably error, but it doesn't *have* to + with rcs as cs: + ... async def test_timeout_deadline_not_on_entry(mock_clock: _core.MockClock) -> None: """Test that not setting timeout_from_enter gives a DeprecationWarning and retains old behaviour.""" - with pytest.warns(DeprecationWarning, match="issues/2512"): - rcs = move_on_after(5) - mock_clock.jump(3) - with rcs as cs: - assert cs.deadline - _core.current_time() == 2 - - with pytest.warns(DeprecationWarning, match="issues/2512"): - cs_gen = fail_after(5) - mock_clock.jump(3) - with cs_gen as cs: - assert cs.deadline - _core.current_time() == 2 + for cs_gen_fun in (move_on_after, fail_after): + with pytest.warns(DeprecationWarning, match="issues/2512"): + rcs = cs_gen_fun(5) + mock_clock.jump(3) + with rcs as cs: + assert rcs.deadline == cs.deadline == _core.current_time() + 2 + + rcs.deadline += 1 + assert rcs.deadline == cs.deadline == _core.current_time() + 3 + + cs.deadline += 1 + assert rcs.deadline == cs.deadline == _core.current_time() + 4 + + +async def test_transitional_functions_fail() -> None: + rcs = move_on_after(5, timeout_from_enter=True) + # these errors are only shown if timeout_from_enter=True + + match_str = "^_RelativeCancelScope does not have `deadline`. You might want `relative_deadline`.$" + with pytest.raises(AttributeError, match=match_str): + assert rcs.deadline + with pytest.raises(AttributeError, match=match_str): + rcs.deadline = 7 + + for prop in "cancelled_caught", "cancel_called": + match_str = f"_RelativeCancelScope does not have `{prop}`, and cannot have been cancelled before entering." + with pytest.raises(AttributeError, match=match_str): + assert getattr(rcs, prop) + + with pytest.raises( + AttributeError, + match=match_str[:36] + "cancel`, and cannot be cancelled before entering.", + ): + rcs.cancel() + + with rcs: + match_str = "^_RelativeCancelScope does not have `{}`. You might want to access the entered `CancelScope`.$" + with pytest.raises(AttributeError, match=match_str.format("deadline")): + assert rcs.deadline + with pytest.raises(AttributeError, match=match_str.format("deadline")): + rcs.deadline = 5 + with pytest.raises(AttributeError, match=match_str.format("cancel")): + rcs.cancel() + with pytest.raises(AttributeError, match=match_str.format("cancelled_caught")): + assert rcs.cancelled_caught + with pytest.raises(AttributeError, match=match_str.format("cancel_called")): + assert rcs.cancel_called + + +async def test_transitional_functions_backwards_compatibility() -> None: + rcs = move_on_after(5) + assert rcs.deadline == 5 + rcs.deadline = 7 + assert rcs.cancelled_caught is False + assert rcs.cancel_called is False + with pytest.raises( + RuntimeError, + match="^It is no longer possible to cancel a relative cancel scope before entering it.$", + ): + rcs.cancel() + + # this does not emit any deprecationwarnings because no time has passed + with rcs as cs: + assert rcs.deadline == cs.deadline + assert abs(rcs.deadline - trio.current_time() - 7) < 0.1 + rcs.cancel() + await trio.lowlevel.checkpoint() + assert rcs.cancelled_caught is cs.cancelled_caught is True + assert rcs.cancel_called is cs.cancel_called is True diff --git a/src/trio/_timeouts.py b/src/trio/_timeouts.py index 8203520f89..097676ed40 100644 --- a/src/trio/_timeouts.py +++ b/src/trio/_timeouts.py @@ -29,8 +29,8 @@ def __init__( shield: bool = False, timeout_from_enter: bool = False, ): - self.relative_deadline = relative_deadline - self.shield = shield + self.relative_deadline: float = relative_deadline + self._shield = shield self._timeout_from_enter = timeout_from_enter self._fail: bool = False @@ -38,6 +38,12 @@ def __init__( self._scope: trio.CancelScope | None = None def __enter__(self) -> trio.CancelScope: + if self._scope is not None: + # This does not have to be the case, but for now we're mirroring the + # behaviour of CancelScope + raise RuntimeError( + "Each _RelativeCancelScope may only be used for a single 'with' block", + ) if ( abs(self._creation_time - trio.current_time()) > 0.01 and not self._timeout_from_enter @@ -64,7 +70,7 @@ def __enter__(self) -> trio.CancelScope: self._scope = trio.CancelScope( deadline=start_time + self.relative_deadline, - shield=self.shield, + shield=self._shield, ) self._scope.__enter__() return self._scope @@ -82,6 +88,111 @@ def __exit__( raise TooSlowError return res + @property + def shield(self) -> bool: + # if self._timeout_from_enter and self._scope is not None: + # we might want to raise an error to force people to use the re-saved CancelScope + # directly, but I'm not sure there is a very strong reason to do that. + if self._scope is None: + return self._shield + return self._scope.shield + + @shield.setter + def shield(self, new_value: bool) -> None: + if self._scope is None: + self._shield = new_value + else: + self._scope.shield = new_value + + @property + def deadline(self) -> float: + """Transitional function to maintain backwards compatibility.""" + if self._timeout_from_enter: + if self._scope is None: + raise AttributeError( + "_RelativeCancelScope does not have `deadline`. You might want `relative_deadline`.", + ) + else: + raise AttributeError( + "_RelativeCancelScope does not have `deadline`. You might want to access the entered `CancelScope`.", + ) + elif self._scope is None: # has not been entered + return self.relative_deadline + else: + return self._scope.deadline + + @deadline.setter + def deadline(self, new_deadline: float) -> None: + """Transitional function to maintain backwards compatibility.""" + if self._timeout_from_enter: + if self._scope is None: + raise AttributeError( + "_RelativeCancelScope does not have `deadline`. You might want `relative_deadline`.", + ) + else: + raise AttributeError( + "_RelativeCancelScope does not have `deadline`. You might want to access the entered `CancelScope`.", + ) + elif self._scope is None: # has not been entered + # we don't raise another deprecationwarning, leaving it for __enter__ + self.relative_deadline = new_deadline + else: + self._scope.deadline = new_deadline + + @property + def cancelled_caught(self) -> bool: + """Transitional function to maintain backwards compatibility.""" + if self._timeout_from_enter: + if self._scope is None: + raise AttributeError( + "_RelativeCancelScope does not have `cancelled_caught`, and cannot have been cancelled before entering.", + ) + else: + raise AttributeError( + "_RelativeCancelScope does not have `cancelled_caught`. You might want to access the entered `CancelScope`.", + ) + elif self._scope is None: + return False + else: + return self._scope.cancelled_caught + + def cancel(self) -> None: + """Transitional function to maintain backwards compatibility.""" + if self._timeout_from_enter: + if self._scope is None: + raise AttributeError( + "_RelativeCancelScope does not have `cancel`, and cannot be cancelled before entering.", + ) + else: + raise AttributeError( + "_RelativeCancelScope does not have `cancel`. You might want to access the entered `CancelScope`.", + ) + elif self._scope is None: + # It may be possible to implement this by immediately canceling the + # created scope in __enter__ + raise RuntimeError( + "It is no longer possible to cancel a relative cancel scope before entering it.", + ) + else: + self._scope.cancel() + + @property + def cancel_called(self) -> bool: + """Transitional function to maintain backwards compatibility.""" + if self._timeout_from_enter: + if self._scope is None: + raise AttributeError( + "_RelativeCancelScope does not have `cancel_called`, and cannot have been cancelled before entering.", + ) + else: + raise AttributeError( + "_RelativeCancelScope does not have `cancel_called`. You might want to access the entered `CancelScope`.", + ) + elif self._scope is None: + return False + else: + return self._scope.cancel_called + def move_on_at(deadline: float, *, shield: bool = False) -> trio.CancelScope: """Use as a context manager to create a cancel scope with the given From a2409481db4c8e8b59e77d26fce9dd7c097219b4 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Thu, 5 Sep 2024 16:19:40 +0200 Subject: [PATCH 14/24] fix test --- src/trio/_tests/test_timeouts.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/trio/_tests/test_timeouts.py b/src/trio/_tests/test_timeouts.py index 6833726b02..9a91e230f7 100644 --- a/src/trio/_tests/test_timeouts.py +++ b/src/trio/_tests/test_timeouts.py @@ -212,8 +212,12 @@ async def test_timeout_deadline_on_entry(mock_clock: _core.MockClock) -> None: assert cs.shield is False # re-entering a _RelativeCancelScope should probably error, but it doesn't *have* to - with rcs as cs: - ... + with pytest.raises( + RuntimeError, + match="^Each _RelativeCancelScope may only be used for a single 'with' block$", + ): + with rcs as cs: + ... async def test_timeout_deadline_not_on_entry(mock_clock: _core.MockClock) -> None: From 0183b4307cb8922096cbce2b1c25eb93208d5bc9 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Thu, 5 Sep 2024 16:43:05 +0200 Subject: [PATCH 15/24] fix tests/codecov --- src/trio/_tests/test_timeouts.py | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/trio/_tests/test_timeouts.py b/src/trio/_tests/test_timeouts.py index 9a91e230f7..18355079da 100644 --- a/src/trio/_tests/test_timeouts.py +++ b/src/trio/_tests/test_timeouts.py @@ -190,6 +190,9 @@ async def test_timeout_deadline_on_entry(mock_clock: _core.MockClock) -> None: rcs = move_on_after(5, timeout_from_enter=True) assert rcs.relative_deadline == 5 + rcs.shield = True + assert rcs.shield + mock_clock.jump(3) start = _core.current_time() with rcs as cs: @@ -203,13 +206,13 @@ async def test_timeout_deadline_on_entry(mock_clock: _core.MockClock) -> None: assert cs.deadline == start + 5 # check that their shield values are linked - assert rcs.shield is False + assert rcs.shield is cs.shield is True - cs.shield = True - assert rcs.shield is True + cs.shield = False + assert rcs.shield is cs.shield is False - rcs.shield = False - assert cs.shield is False + rcs.shield = True + assert rcs.shield is cs.shield is True # re-entering a _RelativeCancelScope should probably error, but it doesn't *have* to with pytest.raises( @@ -289,6 +292,7 @@ async def test_transitional_functions_backwards_compatibility() -> None: assert rcs.deadline == cs.deadline assert abs(rcs.deadline - trio.current_time() - 7) < 0.1 rcs.cancel() - await trio.lowlevel.checkpoint() - assert rcs.cancelled_caught is cs.cancelled_caught is True - assert rcs.cancel_called is cs.cancel_called is True + await trio.lowlevel.checkpoint() # let the cs get cancelled + + assert rcs.cancelled_caught is cs.cancelled_caught is True + assert rcs.cancel_called is cs.cancel_called is True From ca6335482f06ac4b7eb52e69534cc9b7c744002d Mon Sep 17 00:00:00 2001 From: jakkdl Date: Thu, 5 Sep 2024 16:47:05 +0200 Subject: [PATCH 16/24] fix test --- src/trio/_tests/test_timeouts.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/trio/_tests/test_timeouts.py b/src/trio/_tests/test_timeouts.py index 18355079da..32dd7713af 100644 --- a/src/trio/_tests/test_timeouts.py +++ b/src/trio/_tests/test_timeouts.py @@ -190,9 +190,6 @@ async def test_timeout_deadline_on_entry(mock_clock: _core.MockClock) -> None: rcs = move_on_after(5, timeout_from_enter=True) assert rcs.relative_deadline == 5 - rcs.shield = True - assert rcs.shield - mock_clock.jump(3) start = _core.current_time() with rcs as cs: @@ -200,6 +197,9 @@ async def test_timeout_deadline_on_entry(mock_clock: _core.MockClock) -> None: assert cs.deadline == start + 5 rcs = fail_after(5, timeout_from_enter=True) + rcs.shield = True + assert rcs.shield + mock_clock.jump(3) start = _core.current_time() with rcs as cs: From 75fc52b7430d072f1f88ad490d99b65e2ed17288 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Thu, 12 Sep 2024 13:25:27 +0200 Subject: [PATCH 17/24] quick re-implementation after new spec. Docs/docstrings have not had a thorough pass --- src/trio/_core/_run.py | 55 +++++++- src/trio/_tests/test_timeouts.py | 122 +++++------------- src/trio/_timeouts.py | 213 ++----------------------------- 3 files changed, 98 insertions(+), 292 deletions(-) diff --git a/src/trio/_core/_run.py b/src/trio/_core/_run.py index 8921ed7f12..e2893ed8ad 100644 --- a/src/trio/_core/_run.py +++ b/src/trio/_core/_run.py @@ -540,11 +540,21 @@ class CancelScope: _has_been_entered: bool = attrs.field(default=False, init=False) _registered_deadline: float = attrs.field(default=inf, init=False) _cancel_called: bool = attrs.field(default=False, init=False) + _is_relative: bool | None = attrs.field(default=False, init=False) cancelled_caught: bool = attrs.field(default=False, init=False) # Constructor arguments: - _deadline: float = attrs.field(default=inf, kw_only=True, alias="deadline") - _shield: bool = attrs.field(default=False, kw_only=True, alias="shield") + _relative_deadline: float = attrs.field(default=inf, kw_only=True) + _deadline: float = attrs.field(default=inf, kw_only=True) + _shield: bool = attrs.field(default=False, kw_only=True) + + def __attrs_post_init__(self) -> None: + if self._relative_deadline != inf: + if self._deadline != inf: + raise ValueError( + "Cannot specify both a deadline and a relative deadline", + ) + self._is_relative = True @enable_ki_protection def __enter__(self) -> Self: @@ -554,6 +564,12 @@ def __enter__(self) -> Self: "Each CancelScope may only be used for a single 'with' block", ) self._has_been_entered = True + assert self._is_relative is not None + + if self._is_relative: + self._deadline = current_time() + self._relative_deadline + self._is_relative = None + if current_time() >= self._deadline: self.cancel() with self._might_change_registered_deadline(): @@ -734,13 +750,48 @@ def deadline(self) -> float: this can be overridden by the ``deadline=`` argument to the :class:`~trio.CancelScope` constructor. """ + if self._is_relative is True: + raise RuntimeError( + "unentered relative cancel scope does not have an absolute deadline", + ) return self._deadline @deadline.setter def deadline(self, new_deadline: float) -> None: + if self._is_relative is True: + raise RuntimeError( + "unentered relative cancel scope does not have an absolute deadline", + ) with self._might_change_registered_deadline(): self._deadline = float(new_deadline) + @property + def relative_deadline(self) -> float: + if self._is_relative is False: + raise RuntimeError( + "unentered non-relative cancel scope does not have a relative deadline", + ) + elif self._is_relative is None: + return self._deadline - current_time() + return self._relative_deadline + + @relative_deadline.setter + def relative_deadline(self, new_relative_deadline: float) -> None: + if self._is_relative is False: + raise RuntimeError( + "unentered non-relative cancel scope does not have a relative deadline", + ) + elif self._is_relative is True: + self._relative_deadline = new_relative_deadline + else: # entered + with self._might_change_registered_deadline(): + self._deadline = current_time() + float(new_relative_deadline) + + @property + def is_relative(self) -> bool | None: + """Returns None after entering""" + return self._is_relative + @property def shield(self) -> bool: """Read-write, :class:`bool`, default :data:`False`. So long as diff --git a/src/trio/_tests/test_timeouts.py b/src/trio/_tests/test_timeouts.py index 32dd7713af..b7b65000a2 100644 --- a/src/trio/_tests/test_timeouts.py +++ b/src/trio/_tests/test_timeouts.py @@ -187,7 +187,7 @@ async def test_timeouts_raise_value_error() -> None: async def test_timeout_deadline_on_entry(mock_clock: _core.MockClock) -> None: - rcs = move_on_after(5, timeout_from_enter=True) + rcs = move_on_after(5) assert rcs.relative_deadline == 5 mock_clock.jump(3) @@ -195,104 +195,52 @@ async def test_timeout_deadline_on_entry(mock_clock: _core.MockClock) -> None: with rcs as cs: # This would previously be start+2 assert cs.deadline == start + 5 + assert cs.relative_deadline == 5 + + cs.deadline = start + 3 + assert cs.deadline == start + 3 + assert cs.relative_deadline == 3 - rcs = fail_after(5, timeout_from_enter=True) + cs.relative_deadline = 4 + assert cs.deadline == start + 4 + assert cs.relative_deadline == 4 + + rcs = move_on_after(5) + assert rcs.shield is False rcs.shield = True - assert rcs.shield + assert rcs.shield is True mock_clock.jump(3) start = _core.current_time() with rcs as cs: assert cs.deadline == start + 5 - # check that their shield values are linked - assert rcs.shield is cs.shield is True - - cs.shield = False - assert rcs.shield is cs.shield is False - - rcs.shield = True - assert rcs.shield is cs.shield is True - - # re-entering a _RelativeCancelScope should probably error, but it doesn't *have* to - with pytest.raises( - RuntimeError, - match="^Each _RelativeCancelScope may only be used for a single 'with' block$", - ): - with rcs as cs: - ... - - -async def test_timeout_deadline_not_on_entry(mock_clock: _core.MockClock) -> None: - """Test that not setting timeout_from_enter gives a DeprecationWarning and - retains old behaviour.""" - for cs_gen_fun in (move_on_after, fail_after): - with pytest.warns(DeprecationWarning, match="issues/2512"): - rcs = cs_gen_fun(5) - mock_clock.jump(3) - with rcs as cs: - assert rcs.deadline == cs.deadline == _core.current_time() + 2 - - rcs.deadline += 1 - assert rcs.deadline == cs.deadline == _core.current_time() + 3 - - cs.deadline += 1 - assert rcs.deadline == cs.deadline == _core.current_time() + 4 + assert rcs is cs -async def test_transitional_functions_fail() -> None: - rcs = move_on_after(5, timeout_from_enter=True) - # these errors are only shown if timeout_from_enter=True +async def test_invalid_acces_unentered() -> None: + cs = move_on_after(5) - match_str = "^_RelativeCancelScope does not have `deadline`. You might want `relative_deadline`.$" - with pytest.raises(AttributeError, match=match_str): - assert rcs.deadline - with pytest.raises(AttributeError, match=match_str): - rcs.deadline = 7 + match_str = "^unentered relative cancel scope does not have an absolute deadline$" + with pytest.raises(RuntimeError, match=match_str): + assert cs.deadline + with pytest.raises(RuntimeError, match=match_str): + cs.deadline = 7 - for prop in "cancelled_caught", "cancel_called": - match_str = f"_RelativeCancelScope does not have `{prop}`, and cannot have been cancelled before entering." - with pytest.raises(AttributeError, match=match_str): - assert getattr(rcs, prop) + cs = move_on_at(5) - with pytest.raises( - AttributeError, - match=match_str[:36] + "cancel`, and cannot be cancelled before entering.", - ): - rcs.cancel() - - with rcs: - match_str = "^_RelativeCancelScope does not have `{}`. You might want to access the entered `CancelScope`.$" - with pytest.raises(AttributeError, match=match_str.format("deadline")): - assert rcs.deadline - with pytest.raises(AttributeError, match=match_str.format("deadline")): - rcs.deadline = 5 - with pytest.raises(AttributeError, match=match_str.format("cancel")): - rcs.cancel() - with pytest.raises(AttributeError, match=match_str.format("cancelled_caught")): - assert rcs.cancelled_caught - with pytest.raises(AttributeError, match=match_str.format("cancel_called")): - assert rcs.cancel_called - - -async def test_transitional_functions_backwards_compatibility() -> None: - rcs = move_on_after(5) - assert rcs.deadline == 5 - rcs.deadline = 7 - assert rcs.cancelled_caught is False - assert rcs.cancel_called is False - with pytest.raises( - RuntimeError, - match="^It is no longer possible to cancel a relative cancel scope before entering it.$", - ): - rcs.cancel() + match_str = ( + "^unentered non-relative cancel scope does not have a relative deadline$" + ) + with pytest.raises(RuntimeError, match=match_str): + assert cs.relative_deadline + with pytest.raises(RuntimeError, match=match_str): + cs.relative_deadline = 7 - # this does not emit any deprecationwarnings because no time has passed - with rcs as cs: - assert rcs.deadline == cs.deadline - assert abs(rcs.deadline - trio.current_time() - 7) < 0.1 - rcs.cancel() - await trio.lowlevel.checkpoint() # let the cs get cancelled - assert rcs.cancelled_caught is cs.cancelled_caught is True - assert rcs.cancel_called is cs.cancel_called is True +@pytest.mark.xfail(reason="not implemented") +async def test_fail_access_before_entering() -> None: + my_fail_at = fail_at(5) + assert my_fail_at.deadline # type: ignore[attr-defined] + my_fail_after = fail_after(5) + assert my_fail_after.relative_deadline # type: ignore[attr-defined] diff --git a/src/trio/_timeouts.py b/src/trio/_timeouts.py index 097676ed40..dd3a1f01c1 100644 --- a/src/trio/_timeouts.py +++ b/src/trio/_timeouts.py @@ -1,197 +1,13 @@ from __future__ import annotations import math -import warnings from contextlib import contextmanager from typing import TYPE_CHECKING import trio -from ._util import final - if TYPE_CHECKING: from collections.abc import Generator - from types import TracebackType - - -@final -class _RelativeCancelScope: - """Makes it possible to specify relative deadlines at initialization, that does - not start counting until the cm is entered. - Upon entering it returns a CancelScope, so unless initialization and entering - are separate, this class will be transparent to end users. - """ - - def __init__( - self, - relative_deadline: float, - *, - shield: bool = False, - timeout_from_enter: bool = False, - ): - self.relative_deadline: float = relative_deadline - self._shield = shield - self._timeout_from_enter = timeout_from_enter - - self._fail: bool = False - self._creation_time = trio.current_time() - self._scope: trio.CancelScope | None = None - - def __enter__(self) -> trio.CancelScope: - if self._scope is not None: - # This does not have to be the case, but for now we're mirroring the - # behaviour of CancelScope - raise RuntimeError( - "Each _RelativeCancelScope may only be used for a single 'with' block", - ) - if ( - abs(self._creation_time - trio.current_time()) > 0.01 - and not self._timeout_from_enter - ): - # not using warn_deprecated because the message template is a weird fit - # TODO: mention versions in the message? - warnings.warn( - DeprecationWarning( - "`move_on_after` and `fail_after` will change behaviour to " - "start the deadline relative to entering the cm, instead of " - "at creation time. To silence this warning and opt into the " - "new behaviour, pass `timeout_from_enter=True`. " - "To keep old behaviour, use `move_on_at(trio.current_time() + x)` " - "(or `fail_at`), where `x` is the previous timeout length. " - "See https://github.com/python-trio/trio/issues/2512", - ), - stacklevel=2, - ) - - if self._timeout_from_enter: - start_time = trio.current_time() - else: - start_time = self._creation_time - - self._scope = trio.CancelScope( - deadline=start_time + self.relative_deadline, - shield=self._shield, - ) - self._scope.__enter__() - return self._scope - - def __exit__( - self, - exc_type: type[BaseException] | None, - exc_value: BaseException | None, - traceback: TracebackType | None, - ) -> bool | None: - if self._scope is None: # pragma: no cover - raise RuntimeError("__exit__ called before __enter__") - res = self._scope.__exit__(exc_type, exc_value, traceback) - if self._fail and self._scope.cancelled_caught: - raise TooSlowError - return res - - @property - def shield(self) -> bool: - # if self._timeout_from_enter and self._scope is not None: - # we might want to raise an error to force people to use the re-saved CancelScope - # directly, but I'm not sure there is a very strong reason to do that. - if self._scope is None: - return self._shield - return self._scope.shield - - @shield.setter - def shield(self, new_value: bool) -> None: - if self._scope is None: - self._shield = new_value - else: - self._scope.shield = new_value - - @property - def deadline(self) -> float: - """Transitional function to maintain backwards compatibility.""" - if self._timeout_from_enter: - if self._scope is None: - raise AttributeError( - "_RelativeCancelScope does not have `deadline`. You might want `relative_deadline`.", - ) - else: - raise AttributeError( - "_RelativeCancelScope does not have `deadline`. You might want to access the entered `CancelScope`.", - ) - elif self._scope is None: # has not been entered - return self.relative_deadline - else: - return self._scope.deadline - - @deadline.setter - def deadline(self, new_deadline: float) -> None: - """Transitional function to maintain backwards compatibility.""" - if self._timeout_from_enter: - if self._scope is None: - raise AttributeError( - "_RelativeCancelScope does not have `deadline`. You might want `relative_deadline`.", - ) - else: - raise AttributeError( - "_RelativeCancelScope does not have `deadline`. You might want to access the entered `CancelScope`.", - ) - elif self._scope is None: # has not been entered - # we don't raise another deprecationwarning, leaving it for __enter__ - self.relative_deadline = new_deadline - else: - self._scope.deadline = new_deadline - - @property - def cancelled_caught(self) -> bool: - """Transitional function to maintain backwards compatibility.""" - if self._timeout_from_enter: - if self._scope is None: - raise AttributeError( - "_RelativeCancelScope does not have `cancelled_caught`, and cannot have been cancelled before entering.", - ) - else: - raise AttributeError( - "_RelativeCancelScope does not have `cancelled_caught`. You might want to access the entered `CancelScope`.", - ) - elif self._scope is None: - return False - else: - return self._scope.cancelled_caught - - def cancel(self) -> None: - """Transitional function to maintain backwards compatibility.""" - if self._timeout_from_enter: - if self._scope is None: - raise AttributeError( - "_RelativeCancelScope does not have `cancel`, and cannot be cancelled before entering.", - ) - else: - raise AttributeError( - "_RelativeCancelScope does not have `cancel`. You might want to access the entered `CancelScope`.", - ) - elif self._scope is None: - # It may be possible to implement this by immediately canceling the - # created scope in __enter__ - raise RuntimeError( - "It is no longer possible to cancel a relative cancel scope before entering it.", - ) - else: - self._scope.cancel() - - @property - def cancel_called(self) -> bool: - """Transitional function to maintain backwards compatibility.""" - if self._timeout_from_enter: - if self._scope is None: - raise AttributeError( - "_RelativeCancelScope does not have `cancel_called`, and cannot have been cancelled before entering.", - ) - else: - raise AttributeError( - "_RelativeCancelScope does not have `cancel_called`. You might want to access the entered `CancelScope`.", - ) - elif self._scope is None: - return False - else: - return self._scope.cancel_called def move_on_at(deadline: float, *, shield: bool = False) -> trio.CancelScope: @@ -217,17 +33,11 @@ def move_on_after( *, shield: bool = False, timeout_from_enter: bool = False, -) -> _RelativeCancelScope: +) -> trio.CancelScope: """Use as a context manager to create a cancel scope whose deadline is set to now + *seconds*. - The deadline of the cancel scope was previously calculated at creation time, - not upon entering the context manager. This is still the default, but deprecated. - If you pass ``timeout_from_enter=True`` it will instead be calculated relative - to entering the cm, and silence the :class:`DeprecationWarning`. - - If you're entering the cancel scope at initialization time, which is the most common - use case, you can treat this function as returning a :class:`CancelScope`. + The deadline of the cancel scope is calculated upon entering. Args: seconds (float): The timeout. @@ -242,10 +52,9 @@ def move_on_after( raise ValueError("timeout must be non-negative") if math.isnan(seconds): raise ValueError("timeout must not be NaN") - return _RelativeCancelScope( + return trio.CancelScope( shield=shield, relative_deadline=seconds, - timeout_from_enter=timeout_from_enter, ) @@ -338,12 +147,12 @@ def fail_at( raise TooSlowError +@contextmanager def fail_after( seconds: float, *, shield: bool = False, - timeout_from_enter: bool = False, -) -> _RelativeCancelScope: +) -> Generator[trio.CancelScope, None, None]: """Creates a cancel scope with the given timeout, and raises an error if it is actually cancelled. @@ -354,10 +163,7 @@ def fail_after( it's caught and discarded. When it reaches :func:`fail_after`, then it's caught and :exc:`TooSlowError` is raised in its place. - The deadline of the cancel scope was previously calculated at creation time, - not upon entering the context manager. This is still the default, but deprecated. - If you pass ``timeout_from_enter=True`` it will instead be calculated relative - to entering the cm, and silence the :class:`DeprecationWarning`. + The deadline of the cancel scope is calculated upon entering. Args: seconds (float): The timeout. @@ -370,6 +176,7 @@ def fail_after( ValueError: if *seconds* is less than zero or NaN. """ - rcs = move_on_after(seconds, shield=shield, timeout_from_enter=timeout_from_enter) - rcs._fail = True - return rcs + with move_on_after(seconds, shield=shield) as scope: + yield scope + if scope.cancelled_caught: + raise TooSlowError From 5a32eb0ca3c329a534df3c0d45a113377fdbf0ac Mon Sep 17 00:00:00 2001 From: jakkdl Date: Mon, 16 Sep 2024 13:01:23 +0200 Subject: [PATCH 18/24] clean up return type of fail_at/_after for sphinx --- docs/source/reference-core.rst | 35 ---------------------------------- src/trio/_timeouts.py | 12 +++++++++++- 2 files changed, 11 insertions(+), 36 deletions(-) diff --git a/docs/source/reference-core.rst b/docs/source/reference-core.rst index 99e392ecb2..d696f2b63e 100644 --- a/docs/source/reference-core.rst +++ b/docs/source/reference-core.rst @@ -598,41 +598,6 @@ which is sometimes useful: .. autofunction:: current_effective_deadline - -.. _saved_relative_timeouts: - -Separating initialization and entry of a :class:`CancelScope` -+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ -Before v0.26.3 there was no separation of initialization and entry of a :class:`CancelScope`. This meant that trying to initialize :func:`move_on_after` or :func:`fail_after` and entering them at a later point meant that the timeout was relative to initialization, leading to confusing behaviour. - -.. code-block:: python - - # THIS WILL NOW GIVE A DEPRECATION ERROR - my_cs = trio.move_on_after(5) - trio.sleep(1) - with my_cs: # this would move on after 4 seconds - ... - -This is now resolved with the addition of a wrapper class :class:`trio._timeouts._RelativeCancelScope` that is transparent to end users when initializing and entering at the same time. To silence the :class:`DeprecationWarning` and use the new behavior, pass ``timeout_from_enter=True`` to :func:`move_on_after` or :func:`fail_after`. - -.. code-block:: python - - my_cs = trio.move_on_after(5, timeout_from_enter = True) - trio.sleep(1) - with my_cs: # this will now move on after 5 seconds - ... - -If you want to retain previous behaviour, you should now instead write something like - -.. code-block:: python - - my_cs = trio.move_on_at(trio.current_time() + 5) - trio.sleep(1) - with my_cs: # this will now unambiguously move on after 4 seconds - ... - -.. autoclass:: trio._timeouts._RelativeCancelScope - .. _tasks: Tasks let you do multiple things at once diff --git a/src/trio/_timeouts.py b/src/trio/_timeouts.py index dd3a1f01c1..6f470c130b 100644 --- a/src/trio/_timeouts.py +++ b/src/trio/_timeouts.py @@ -1,6 +1,7 @@ from __future__ import annotations import math +import sys from contextlib import contextmanager from typing import TYPE_CHECKING @@ -32,7 +33,6 @@ def move_on_after( seconds: float, *, shield: bool = False, - timeout_from_enter: bool = False, ) -> trio.CancelScope: """Use as a context manager to create a cancel scope whose deadline is set to now + *seconds*. @@ -180,3 +180,13 @@ def fail_after( yield scope if scope.cancelled_caught: raise TooSlowError + + +# Users don't need to know that fail_at & fail_after wraps move_on_at and move_on_after +# and there is no functional difference. So we replace the return value when generating +# documentation. +if "sphinx" in sys.modules: + import inspect + + for c in (fail_at, fail_after): + c.__signature__ = inspect.Signature.from_callable(c).replace(return_annotation=trio.CancelScope) # type: ignore[union-attr] From c2a3d8ef0de16ad2592aab0559e55ec6e2b14b35 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Mon, 16 Sep 2024 13:32:32 +0200 Subject: [PATCH 19/24] remove _is_relative, and calculate it on the fly instead. Add nan handling to CancelScope. Move value validation from move_on_after/at to CancelScope --- src/trio/_core/_run.py | 61 ++++++++++++++++++++++++++---------------- src/trio/_timeouts.py | 7 ----- 2 files changed, 38 insertions(+), 30 deletions(-) diff --git a/src/trio/_core/_run.py b/src/trio/_core/_run.py index e2893ed8ad..5a7736355a 100644 --- a/src/trio/_core/_run.py +++ b/src/trio/_core/_run.py @@ -13,7 +13,7 @@ from contextlib import AbstractAsyncContextManager, contextmanager, suppress from contextvars import copy_context from heapq import heapify, heappop, heappush -from math import inf +from math import inf, isnan from time import perf_counter from typing import ( TYPE_CHECKING, @@ -540,7 +540,6 @@ class CancelScope: _has_been_entered: bool = attrs.field(default=False, init=False) _registered_deadline: float = attrs.field(default=inf, init=False) _cancel_called: bool = attrs.field(default=False, init=False) - _is_relative: bool | None = attrs.field(default=False, init=False) cancelled_caught: bool = attrs.field(default=False, init=False) # Constructor arguments: @@ -549,12 +548,16 @@ class CancelScope: _shield: bool = attrs.field(default=False, kw_only=True) def __attrs_post_init__(self) -> None: - if self._relative_deadline != inf: - if self._deadline != inf: - raise ValueError( - "Cannot specify both a deadline and a relative deadline", - ) - self._is_relative = True + if isnan(self._deadline): + raise ValueError("deadline must not be NaN") + if isnan(self._relative_deadline): + raise ValueError("relative deadline must not be NaN") + if self._relative_deadline < 0: + raise ValueError("timeout must be non-negative") + if self._relative_deadline != inf and self._deadline != inf: + raise ValueError( + "Cannot specify both a deadline and a relative deadline", + ) @enable_ki_protection def __enter__(self) -> Self: @@ -564,11 +567,11 @@ def __enter__(self) -> Self: "Each CancelScope may only be used for a single 'with' block", ) self._has_been_entered = True - assert self._is_relative is not None - if self._is_relative: + if self._relative_deadline != inf: + assert self._deadline == inf self._deadline = current_time() + self._relative_deadline - self._is_relative = None + self._relative_deadline = inf if current_time() >= self._deadline: self.cancel() @@ -750,7 +753,8 @@ def deadline(self) -> float: this can be overridden by the ``deadline=`` argument to the :class:`~trio.CancelScope` constructor. """ - if self._is_relative is True: + if self._relative_deadline != inf: + assert self._deadline == inf raise RuntimeError( "unentered relative cancel scope does not have an absolute deadline", ) @@ -758,7 +762,10 @@ def deadline(self) -> float: @deadline.setter def deadline(self, new_deadline: float) -> None: - if self._is_relative is True: + if isnan(new_deadline): + raise ValueError("deadline must not be NaN") + if self._relative_deadline != inf: + assert self._deadline == inf raise RuntimeError( "unentered relative cancel scope does not have an absolute deadline", ) @@ -767,30 +774,38 @@ def deadline(self, new_deadline: float) -> None: @property def relative_deadline(self) -> float: - if self._is_relative is False: + if self._has_been_entered: + return self._deadline - current_time() + elif self._deadline != inf: + assert self._relative_deadline == inf raise RuntimeError( "unentered non-relative cancel scope does not have a relative deadline", ) - elif self._is_relative is None: - return self._deadline - current_time() return self._relative_deadline @relative_deadline.setter def relative_deadline(self, new_relative_deadline: float) -> None: - if self._is_relative is False: + if isnan(new_relative_deadline): + raise ValueError("deadline must not be NaN") + if self._has_been_entered: + with self._might_change_registered_deadline(): + self._deadline = current_time() + float(new_relative_deadline) + elif self._deadline != inf: + assert self._relative_deadline == inf raise RuntimeError( "unentered non-relative cancel scope does not have a relative deadline", ) - elif self._is_relative is True: + else: self._relative_deadline = new_relative_deadline - else: # entered - with self._might_change_registered_deadline(): - self._deadline = current_time() + float(new_relative_deadline) @property def is_relative(self) -> bool | None: - """Returns None after entering""" - return self._is_relative + """Returns None after entering. Returns False if both deadline and + relative_deadline are inf.""" + assert not (self._deadline != inf and self._relative_deadline != inf) + if self._has_been_entered: + return None + return self._deadline != inf @property def shield(self) -> bool: diff --git a/src/trio/_timeouts.py b/src/trio/_timeouts.py index 6f470c130b..d1bc9fafb9 100644 --- a/src/trio/_timeouts.py +++ b/src/trio/_timeouts.py @@ -1,6 +1,5 @@ from __future__ import annotations -import math import sys from contextlib import contextmanager from typing import TYPE_CHECKING @@ -24,8 +23,6 @@ def move_on_at(deadline: float, *, shield: bool = False) -> trio.CancelScope: ValueError: if deadline is NaN. """ - if math.isnan(deadline): - raise ValueError("deadline must not be NaN") return trio.CancelScope(deadline=deadline, shield=shield) @@ -48,10 +45,6 @@ def move_on_after( ValueError: if timeout is less than zero or NaN. """ - if seconds < 0: - raise ValueError("timeout must be non-negative") - if math.isnan(seconds): - raise ValueError("timeout must not be NaN") return trio.CancelScope( shield=shield, relative_deadline=seconds, From f8e94171ae0a1850cdabd8bd7f7f58d31165490e Mon Sep 17 00:00:00 2001 From: jakkdl Date: Mon, 16 Sep 2024 15:18:13 +0200 Subject: [PATCH 20/24] change some RuntimeError to DeprecationWarning, fix tests/coverage --- src/trio/_core/_run.py | 22 ++++++++++++++++------ src/trio/_core/_tests/test_run.py | 23 ++++++++++++++++++++++- src/trio/_tests/test_timeouts.py | 24 ++++++++++++++++++------ 3 files changed, 56 insertions(+), 13 deletions(-) diff --git a/src/trio/_core/_run.py b/src/trio/_core/_run.py index 5a7736355a..5de75e8e6c 100644 --- a/src/trio/_core/_run.py +++ b/src/trio/_core/_run.py @@ -755,9 +755,13 @@ def deadline(self) -> float: """ if self._relative_deadline != inf: assert self._deadline == inf - raise RuntimeError( - "unentered relative cancel scope does not have an absolute deadline", + warnings.warn( + DeprecationWarning( + "unentered relative cancel scope does not have an absolute deadline. Use `.relative_deadline`", + ), + stacklevel=2, ) + return current_time() + self._relative_deadline return self._deadline @deadline.setter @@ -766,9 +770,13 @@ def deadline(self, new_deadline: float) -> None: raise ValueError("deadline must not be NaN") if self._relative_deadline != inf: assert self._deadline == inf - raise RuntimeError( - "unentered relative cancel scope does not have an absolute deadline", + warnings.warn( + DeprecationWarning( + "unentered relative cancel scope does not have an absolute deadline. Transforming into an absolute cancel scope. First set `.relative_deadline = math.inf` if you do want an absolute cancel scope.", + ), + stacklevel=2, ) + self._relative_deadline = inf with self._might_change_registered_deadline(): self._deadline = float(new_deadline) @@ -786,7 +794,9 @@ def relative_deadline(self) -> float: @relative_deadline.setter def relative_deadline(self, new_relative_deadline: float) -> None: if isnan(new_relative_deadline): - raise ValueError("deadline must not be NaN") + raise ValueError("relative deadline must not be NaN") + if new_relative_deadline < 0: + raise ValueError("relative deadline must be non-negative") if self._has_been_entered: with self._might_change_registered_deadline(): self._deadline = current_time() + float(new_relative_deadline) @@ -805,7 +815,7 @@ def is_relative(self) -> bool | None: assert not (self._deadline != inf and self._relative_deadline != inf) if self._has_been_entered: return None - return self._deadline != inf + return self._relative_deadline != inf @property def shield(self) -> bool: diff --git a/src/trio/_core/_tests/test_run.py b/src/trio/_core/_tests/test_run.py index e6ee5e869f..9d2eab787d 100644 --- a/src/trio/_core/_tests/test_run.py +++ b/src/trio/_core/_tests/test_run.py @@ -9,7 +9,7 @@ import types import weakref from contextlib import ExitStack, contextmanager, suppress -from math import inf +from math import inf, nan from typing import TYPE_CHECKING, Any, NoReturn, TypeVar, cast import outcome @@ -365,6 +365,27 @@ async def test_cancel_scope_repr(mock_clock: _core.MockClock) -> None: assert "exited" in repr(scope) +async def test_cancel_scope_validation() -> None: + with pytest.raises( + ValueError, + match="^Cannot specify both a deadline and a relative deadline$", + ): + _core.CancelScope(deadline=7, relative_deadline=3) + scope = _core.CancelScope() + + with pytest.raises(ValueError, match="^deadline must not be NaN$"): + scope.deadline = nan + with pytest.raises(ValueError, match="^relative deadline must not be NaN$"): + scope.relative_deadline = nan + + with pytest.raises(ValueError, match="^relative deadline must be non-negative$"): + scope.relative_deadline = -3 + scope.relative_deadline = 5 + assert scope.relative_deadline == 5 + + # several related tests of CancelScope are implicitly handled by test_timeouts.py + + def test_cancel_points() -> None: async def main1() -> None: with _core.CancelScope() as scope: diff --git a/src/trio/_tests/test_timeouts.py b/src/trio/_tests/test_timeouts.py index b7b65000a2..5add7cbe8c 100644 --- a/src/trio/_tests/test_timeouts.py +++ b/src/trio/_tests/test_timeouts.py @@ -180,7 +180,7 @@ async def test_timeouts_raise_value_error() -> None: ): with pytest.raises( ValueError, - match="^(duration|deadline|timeout) must (not )*be (non-negative|NaN)$", + match="^(duration|deadline|timeout|relative deadline) must (not )*be (non-negative|NaN)$", ): with cm(val): pass # pragma: no cover @@ -193,6 +193,8 @@ async def test_timeout_deadline_on_entry(mock_clock: _core.MockClock) -> None: mock_clock.jump(3) start = _core.current_time() with rcs as cs: + assert cs.is_relative is None + # This would previously be start+2 assert cs.deadline == start + 5 assert cs.relative_deadline == 5 @@ -218,14 +220,24 @@ async def test_timeout_deadline_on_entry(mock_clock: _core.MockClock) -> None: assert rcs is cs -async def test_invalid_acces_unentered() -> None: +async def test_invalid_access_unentered(mock_clock: _core.MockClock) -> None: cs = move_on_after(5) + mock_clock.jump(3) + start = _core.current_time() - match_str = "^unentered relative cancel scope does not have an absolute deadline$" - with pytest.raises(RuntimeError, match=match_str): - assert cs.deadline - with pytest.raises(RuntimeError, match=match_str): + match_str = "^unentered relative cancel scope does not have an absolute deadline" + with pytest.warns(DeprecationWarning, match=match_str): + assert cs.deadline == start + 5 + mock_clock.jump(1) + # this is hella sketchy, but they *have* been warned + with pytest.warns(DeprecationWarning, match=match_str): + assert cs.deadline == start + 6 + + with pytest.warns(DeprecationWarning, match=match_str): cs.deadline = 7 + # now transformed into absolute + assert cs.deadline == 7 + assert not cs.is_relative cs = move_on_at(5) From 36786a66a804ec8d9fc59a97353ef127e981f9d4 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Mon, 16 Sep 2024 15:39:41 +0200 Subject: [PATCH 21/24] pragma: no cover --- src/trio/_tests/test_timeouts.py | 2 +- src/trio/_timeouts.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/trio/_tests/test_timeouts.py b/src/trio/_tests/test_timeouts.py index 5add7cbe8c..27711f0422 100644 --- a/src/trio/_tests/test_timeouts.py +++ b/src/trio/_tests/test_timeouts.py @@ -251,7 +251,7 @@ async def test_invalid_access_unentered(mock_clock: _core.MockClock) -> None: @pytest.mark.xfail(reason="not implemented") -async def test_fail_access_before_entering() -> None: +async def test_fail_access_before_entering() -> None: # pragma: no cover my_fail_at = fail_at(5) assert my_fail_at.deadline # type: ignore[attr-defined] my_fail_after = fail_after(5) diff --git a/src/trio/_timeouts.py b/src/trio/_timeouts.py index d1bc9fafb9..3a56262520 100644 --- a/src/trio/_timeouts.py +++ b/src/trio/_timeouts.py @@ -178,7 +178,7 @@ def fail_after( # Users don't need to know that fail_at & fail_after wraps move_on_at and move_on_after # and there is no functional difference. So we replace the return value when generating # documentation. -if "sphinx" in sys.modules: +if "sphinx" in sys.modules: # pragma: no cover import inspect for c in (fail_at, fail_after): From 6a7650df7cdc4dc46608c2bbdcf71d9379dc2313 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Wed, 18 Sep 2024 12:28:29 +0200 Subject: [PATCH 22/24] duplicate validation logic, bump pyright --- src/trio/_tests/test_timeouts.py | 4 ++-- src/trio/_timeouts.py | 11 +++++++++-- test-requirements.txt | 2 +- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/trio/_tests/test_timeouts.py b/src/trio/_tests/test_timeouts.py index 27711f0422..515f536c1f 100644 --- a/src/trio/_tests/test_timeouts.py +++ b/src/trio/_tests/test_timeouts.py @@ -166,7 +166,7 @@ async def test_timeouts_raise_value_error() -> None: ): with pytest.raises( ValueError, - match="^(duration|deadline|timeout) must (not )*be (non-negative|NaN)$", + match="^(deadline|`seconds`) must (not )*be (non-negative|NaN)$", ): await fun(val) @@ -180,7 +180,7 @@ async def test_timeouts_raise_value_error() -> None: ): with pytest.raises( ValueError, - match="^(duration|deadline|timeout|relative deadline) must (not )*be (non-negative|NaN)$", + match="^(deadline|`seconds`) must (not )*be (non-negative|NaN)$", ): with cm(val): pass # pragma: no cover diff --git a/src/trio/_timeouts.py b/src/trio/_timeouts.py index 3a56262520..6673389558 100644 --- a/src/trio/_timeouts.py +++ b/src/trio/_timeouts.py @@ -1,5 +1,6 @@ from __future__ import annotations +import math import sys from contextlib import contextmanager from typing import TYPE_CHECKING @@ -23,6 +24,7 @@ def move_on_at(deadline: float, *, shield: bool = False) -> trio.CancelScope: ValueError: if deadline is NaN. """ + # CancelScope validates that deadline isn't math.nan return trio.CancelScope(deadline=deadline, shield=shield) @@ -42,9 +44,14 @@ def move_on_after( of the newly created cancel scope. Raises: - ValueError: if timeout is less than zero or NaN. + ValueError: if `seconds` is less than zero or NaN. """ + # duplicate validation logic to have the correct parameter name + if seconds < 0: + raise ValueError("`seconds` must be non-negative") + if math.isnan(seconds): + raise ValueError("`seconds` must not be NaN") return trio.CancelScope( shield=shield, relative_deadline=seconds, @@ -92,7 +99,7 @@ async def sleep(seconds: float) -> None: """ if seconds < 0: - raise ValueError("duration must be non-negative") + raise ValueError("`seconds` must be non-negative") if seconds == 0: await trio.lowlevel.checkpoint() else: diff --git a/test-requirements.txt b/test-requirements.txt index 47aea97d8f..1173aff405 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -105,7 +105,7 @@ pylint==3.2.7 # via -r test-requirements.in pyopenssl==24.2.1 # via -r test-requirements.in -pyright==1.1.378 +pyright==1.1.381 # via -r test-requirements.in pytest==8.3.2 # via -r test-requirements.in From bf12a8011ce4dbb558d2a643cd09f5ccf2a35951 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Wed, 18 Sep 2024 13:24:40 +0200 Subject: [PATCH 23/24] update docs/newsfragments/docstrings --- docs/source/conf.py | 1 + docs/source/reference-core.rst | 4 ++++ newsfragments/2512.breaking.rst | 2 ++ newsfragments/2512.deprecated.rst | 1 - newsfragments/2512.feature.rst | 2 +- src/trio/_timeouts.py | 2 +- 6 files changed, 9 insertions(+), 3 deletions(-) create mode 100644 newsfragments/2512.breaking.rst delete mode 100644 newsfragments/2512.deprecated.rst diff --git a/docs/source/conf.py b/docs/source/conf.py index ecdbdd4701..2e84d91532 100755 --- a/docs/source/conf.py +++ b/docs/source/conf.py @@ -229,6 +229,7 @@ def setup(app: Sphinx) -> None: "pyopenssl": ("https://www.pyopenssl.org/en/stable/", None), "sniffio": ("https://sniffio.readthedocs.io/en/latest/", None), "trio-util": ("https://trio-util.readthedocs.io/en/latest/", None), + "flake8-async": ("https://flake8-async.readthedocs.io/en/latest/", None), } # See https://sphinx-hoverxref.readthedocs.io/en/latest/configuration.html diff --git a/docs/source/reference-core.rst b/docs/source/reference-core.rst index d696f2b63e..4b023292e9 100644 --- a/docs/source/reference-core.rst +++ b/docs/source/reference-core.rst @@ -527,8 +527,12 @@ objects. .. autoattribute:: deadline + .. autoattribute:: relative_deadline + .. autoattribute:: shield + .. automethod:: is_relative() + .. automethod:: cancel() .. attribute:: cancelled_caught diff --git a/newsfragments/2512.breaking.rst b/newsfragments/2512.breaking.rst new file mode 100644 index 0000000000..ef91f684bb --- /dev/null +++ b/newsfragments/2512.breaking.rst @@ -0,0 +1,2 @@ +:func:`trio.move_on_after` and :func:`trio.fail_after` previously set the deadline relative to initialization time, instead of more intuitively upon entering the context manager. This might change timeouts if a program relied on this behavior. If you want to restore previous behavior you should instead use ``trio.move_on_at(trio.current_time() + ...)``. +flake8-async has a new rule to catch this, in case you're supporting older trio versions. See :ref:`ASYNC121` (TODO: except it will be ASYNC122, I'm just making sure intersphinx work) diff --git a/newsfragments/2512.deprecated.rst b/newsfragments/2512.deprecated.rst deleted file mode 100644 index bbb098b1c8..0000000000 --- a/newsfragments/2512.deprecated.rst +++ /dev/null @@ -1 +0,0 @@ -:func:`trio.move_on_after` and :func:`trio.fail_after` previously set the deadline relative to initialization time, instead of more intuitively upon entering the context manager. Any code where these two points in time do not line up (according to :func:`trio.current_time`) will now issue a `DeprecationWarning`. To silence the warning and adopt the new behaviour, pass ``timeout_from_enter = True``. For more information see :ref:`saved_relative_timeouts`. diff --git a/newsfragments/2512.feature.rst b/newsfragments/2512.feature.rst index d30c891961..fe9197cde5 100644 --- a/newsfragments/2512.feature.rst +++ b/newsfragments/2512.feature.rst @@ -1 +1 @@ -:func:`trio.move_on_after` and :func:`trio.fail_after` now allows setting the deadline to be relative to entering the context manager. This will be the default in the future, and to opt into the new behaviour you can pass ``timeout_from_enter=True``. This does not matter in the vast majority of cases where initialization and entering of the context manager is in the same place, and those cases will not raise any :class:`DeprecationWarning`. For more information see :ref:`saved_relative_timeouts`. +:meth:`CancelScope.relative_deadline` and :meth:`CancelScope.is_relative` added, as well as a ``relative_deadline`` parameter to ``__init__``. This allows initializing scopes ahead of time, but where the specified relative deadline doesn't count down until the scope is entered. diff --git a/src/trio/_timeouts.py b/src/trio/_timeouts.py index 6673389558..1a5e2e1db3 100644 --- a/src/trio/_timeouts.py +++ b/src/trio/_timeouts.py @@ -44,7 +44,7 @@ def move_on_after( of the newly created cancel scope. Raises: - ValueError: if `seconds` is less than zero or NaN. + ValueError: if ``seconds`` is less than zero or NaN. """ # duplicate validation logic to have the correct parameter name From 9961abcef5a65dd3ed4f849f880337a5d827de0f Mon Sep 17 00:00:00 2001 From: jakkdl Date: Thu, 19 Sep 2024 13:03:27 +0200 Subject: [PATCH 24/24] properly link to ASYNC122 --- newsfragments/2512.breaking.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/newsfragments/2512.breaking.rst b/newsfragments/2512.breaking.rst index ef91f684bb..f9a6b42631 100644 --- a/newsfragments/2512.breaking.rst +++ b/newsfragments/2512.breaking.rst @@ -1,2 +1,2 @@ :func:`trio.move_on_after` and :func:`trio.fail_after` previously set the deadline relative to initialization time, instead of more intuitively upon entering the context manager. This might change timeouts if a program relied on this behavior. If you want to restore previous behavior you should instead use ``trio.move_on_at(trio.current_time() + ...)``. -flake8-async has a new rule to catch this, in case you're supporting older trio versions. See :ref:`ASYNC121` (TODO: except it will be ASYNC122, I'm just making sure intersphinx work) +flake8-async has a new rule to catch this, in case you're supporting older trio versions. See :ref:`ASYNC122`.