-
-
Notifications
You must be signed in to change notification settings - Fork 363
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
Changes from 1 commit
dea2e79
d6e91cc
dc3bd3d
da25949
7a8262a
d028037
5447f45
1381560
742c013
0075b56
9d3d0b9
b29db0d
480681d
00ba1ca
510aaa1
7079d71
3474128
df5876d
a240948
0183b43
ca63354
7591297
75fc52b
5a32eb0
c2a3d8e
f8e9417
36786a6
8e863b8
6a7650d
02ac588
bf12a80
9961abc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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) | ||
jakkdl marked this conversation as resolved.
Show resolved
Hide resolved
|
||
... | ||
with cs: | ||
... | ||
jakkdl marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
|
||
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: | ||
... |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 | ||
... |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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" | ||
) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can maybe imagine something, but it feels niche and almost certainly could be recreated with two nested scopes or custom functions. But if we don't want simultaneous usage, and also don't want relative_deadline usage after entering, then I think we should go with a different approach than adding an attribute to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. cc @Zac-HD because you have opinions. I think what @Zac-HD proposed of disallowing setting relative deadlines inside the cancel scope would help somewhat, even though I still think that absolute deadlines vs relative deadlines is a weird concept to have before the context manager. (So namely being able to pass both here and being able to set both with their properties) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My preferred semantics are:
The existing deprecation logic is a bit ugly, but seems like the best option available overall. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this your preferred semantics? I'm mostly just seeing downsides to having the feature be this hidden, but still able to sneakily introduce confusing behavior if a user is sending around a If we don't want to introduce relative deadlines as a primary feature, and avoid any stealthy gotchas, then I strongly think we should introduce a class that transforms into a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, the cs = move_on_after(5)
# wait no I want to make the deadline later
cs.deadline = trio.current_time() + 7
with cs:
print(cs.deadline - trio.current_time()) # prints 5 this is regardless of if the time is relative to construction or entering. and of course cs = move_on_after(5)
cs.deadline = cs.deadline + 2
with cs:
print(cs.deadline) # prints 5, not 7, which it would previously There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I now agree that my proposal upthread is clearly bad 😅 |
||
_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() | ||
jakkdl marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
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 | ||
jakkdl marked this conversation as resolved.
Show resolved
Hide resolved
|
||
else: | ||
self._deadline = current_time() + relative_deadline | ||
|
||
@property | ||
def shield(self) -> bool: | ||
"""Read-write, :class:`bool`, default :data:`False`. So long as | ||
|
Uh oh!
There was an error while loading. Please reload this page.