8000 Change `fail_after`&`move_on_after` to set deadline relative to entering. Add CancelScope.relative_deadline by jakkdl · Pull Request #3010 · python-trio/trio · GitHub
[go: up one dir, main page]

Skip to content

Change fail_after&move_on_after to set deadline relative to entering. Add CancelScope.relative_deadline #3010

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 32 commits into from
Sep 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
dea2e79
flake8-trio has been renamed
jakkdl May 22, 2024
d6e91cc
Add explicit documentation on timeout semantic for fail&move_on_after
jakkdl Jun 6, 2024
dc3bd3d
Merge branch 'master' into fail_after_documentation
jakkdl Jun 6, 2024
da25949
use a5rocks alternate phrasing for move_on_after
jakkdl Jun 7, 2024
7a8262a
add `relative_deadline` attribute to CancelScope. `move_on_after` and…
jakkdl Jun 20, 2024
d028037
Merge branch 'master' into fail_after_documentation
jakkdl Jun 20, 2024
5447f45
fix codecov, mypy now fails to unify return types across the timeouts…
jakkdl Jun 20, 2024
1381560
instead of a breaking change we now raise deprecationwarning and give…
jakkdl Jun 24, 2024
742c013
don't inherit from AbstractContextManager since that breaks 3.8, and …
jakkdl Jun 24, 2024
0075b56
fix RTD build. Replace star import with explicit list. Remove useless…
jakkdl Jun 24, 2024
9d3d0b9
reimplement as transforming class
jakkdl Jun 26, 2024
b29db0d
Merge branch 'master' into fail_after_documentation
jakkdl Jun 27, 2024
480681d
whoops, forgot to actually enter the cancelscope
jakkdl Jun 27, 2024
00ba1ca
Merge remote-tracking branch 'origin/main' into fail_after_documentation
jakkdl Aug 31, 2024
510aaa1
remove unneeded type:ignore with mypy 1.11
jakkdl Aug 31, 2024
7079d71
Merge branch 'main' into fail_after_documentation
jakkdl Sep 5, 2024
3474128
write docs, update newsfragments to match current implementation
jakkdl Sep 5, 2024
df5876d
add transitional functions
jakkdl Sep 5, 2024
a240948
fix test
jakkdl Sep 5, 2024
0183b43
fix tests/codecov
jakkdl Sep 5, 2024
ca63354
fix test
jakkdl Sep 5, 2024
7591297
Merge remote-tracking branch 'origin/main' into repro_pyright_verifyt…
jakkdl Sep 12, 2024
75fc52b
quick re-implementation after new spec. Docs/docstrings have not had …
jakkdl Sep 12, 2024
5a32eb0
clean up return type of fail_at/_after for sphinx
jakkdl Sep 16, 2024
c2a3d8e
remove _is_relative, and calculate it on the fly instead. Add nan han…
jakkdl Sep 16, 2024
f8e9417
change some RuntimeError to DeprecationWarning, fix tests/coverage
jakkdl Sep 16, 2024
36786a6
pragma: no cover
jakkdl Sep 16, 2024
8e863b8
Merge remote-tracking branch 'origin/main' into fail_after_documentation
jakkdl Sep 16, 2024
6a7650d
duplicate validation logic, bump pyright
jakkdl Sep 18, 2024
02ac588
Merge remote-tracking branch 'origin/main' into fail_after_documentation
jakkdl Sep 18, 2024
bf12a80
update docs/newsfragments/docstrings
jakkdl Sep 18, 2024
9961abc
properly link to ASYNC122
jakkdl Sep 19, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/source/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 6 additions & 2 deletions docs/source/reference-core.rst
Original file line number Diff line number Diff line change
Expand Up @@ -527,8 +527,12 @@ objects.

.. autoattribute:: deadline

.. autoattribute:: relative_deadline

.. autoattribute:: shield

.. automethod:: is_relative()

.. automethod:: cancel()

.. attribute:: cancelled_caught
Expand Down Expand Up @@ -561,7 +565,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:
Expand Down Expand Up @@ -597,7 +602,6 @@ which is sometimes useful:

.. autofunction:: current_effective_deadline


.. _tasks:

Tasks let you do multiple things at once
Expand Down
2 changes: 2 additions & 0 deletions newsfragments/2512.breaking.rst
Original file line number Diff line number Diff line change
@@ -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:`ASYNC122`.
1 change: 1 addition & 0 deletions newsfragments/2512.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
: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.
82 changes: 79 additions & 3 deletions src/trio/_core/_run.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -543,8 +543,21 @@
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 isnan(self._deadline):
raise ValueError("deadline must not be NaN")
if isnan(self._relative_deadline):
raise ValueError("relative deadline must not be NaN")

Check warning on line 554 in src/trio/_core/_run.py

View check run for this annotation

Codecov / codecov/patch

src/trio/_core/_run.py#L554

Added line #L554 was not covered by tests
if self._relative_deadline < 0:
raise ValueError("timeout must be non-negative")

Check warning on line 556 in src/trio/_core/_run.py

View check run for this annotation

Codecov / codecov/patch

src/trio/_core/_run.py#L556

Added line #L556 was not covered by tests
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:
Expand All @@ -554,6 +567,12 @@
"Each CancelScope may only be used for a single 'with' block",
)
self._has_been_entered = True

if self._relative_deadline != inf:
assert self._deadline == inf
self._deadline = current_time() + self._relative_deadline
self._relative_deadline = inf

if current_time() >= self._deadline:
self.cancel()
with self._might_change_registered_deadline():
Expand Down Expand Up @@ -734,13 +753,70 @@
this can be overridden by the ``deadline=`` argument to
the :class:`~trio.CancelScope` constructor.
"""
if self._relative_deadline != inf:
assert self._deadline == inf
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
def deadline(self, new_deadline: float) -> None:
if isnan(new_deadline):
raise ValueError("deadline must not be NaN")
if self._relative_deadline != inf:
assert self._deadline == inf
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)

@property
def relative_deadline(self) -> float:
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",
)
return self._relative_deadline

@relative_deadline.setter
def relative_deadline(self, new_relative_deadline: float) -> None:
if isnan(new_relative_deadline):
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)
elif self._deadline != inf:
assert self._relative_deadline == inf
raise RuntimeError(
"unentered non-relative cancel scope does not have a relative deadline",
)
else:
self._relative_deadline = new_relative_deadline

@property
def is_relative(self) -> bool | None:
"""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._relative_deadline != inf

@property
def shield(self) -> bool:
"""Read-write, :class:`bool`, default :data:`False`. So long as
Expand Down
23 changes: 22 additions & 1 deletion src/trio/_core/_tests/test_run.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
89 changes: 86 additions & 3 deletions src/trio/_tests/test_timeouts.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,20 @@
import outcome
import pytest

import trio

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_forever,
sleep_until,
)
from ..testing import assert_checkpoints

T = TypeVar("T")
Expand Down Expand Up @@ -155,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)

Expand All @@ -169,7 +180,79 @@ 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)$",
):
with cm(val):
pass # pragma: no cover


async def test_timeout_deadline_on_entry(mock_clock: _core.MockClock) -> None:
rcs = move_on_after(5)
assert rcs.relative_deadline == 5

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

cs.deadline = start + 3
assert cs.deadline == start + 3
assert cs.relative_deadline == 3

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 is True

mock_clock.jump(3)
start = _core.current_time()
with rcs as cs:
assert cs.deadline == start + 5

assert rcs is cs


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.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)

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


@pytest.mark.xfail(reason="not implemented")
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)
assert my_fail_after.relative_deadline # type: ignore[attr-defined]
Loading
Loading
0