move_on_ and fail_ functions accepts shield kwarg

[agnesnatasya]

make move_on_at, move_on_after, fail_at and fail_after accepts shield as a kwarg.

Closes #3052

[codecov]

Codecov Report

Attention: Patch coverage is 94.11765% with 2 lines in your changes missing coverage. Please review.

Project coverage is 99.62%. Comparing base (8e38a1e) to head (2a998f2).
Report is 1 commits behind head on main.

Files	Patch %	Lines
src/trio/_tests/test_timeouts.py	92.30%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3051      +/-   ##
==========================================
- Coverage   99.63%   99.62%   -0.02%     
==========================================
  Files         120      120              
  Lines       17832    17857      +25     
  Branches     3204     3210       +6     
==========================================
+ Hits        17767    17790      +23     
- Misses         46       48       +2     
  Partials       19       19              

Files	Coverage Δ
src/trio/_timeouts.py	100.00% <100.00%> (ø)
src/trio/_tests/test_timeouts.py	97.64% <92.30%> (-2.36%)	⬇️

[A5rocks]

Just a comment about the tests, I don't have much of an opinion on the feature itself.

[A5rocks]

Given that this is using sleep a checkpoint, can you use a checkpoint (or trio.sleep(0)) instead of a hardcoded small value?

[agnesnatasya]

If i use 0, i don't think i can make use of check_takes_about because it checks dur / expected_dur < 1.5, and expected_dur = 0 in this case. The docstring also says that they observe overruns when they sleep for 0.05s, so I picked 0.1s here.
Alternatively, I can use a different way to test whether we actually sleep (instead of using check_takes_about)

[oremanj]

So don't use check_takes_about (and don't mark this test as slow). Instead do something like:

try:
    await trio.lowlevel.checkpoint()
except trio.Cancelled:
    pytest.fail("shield didn't work")
inner.shield = False
with pytest.raises(trio.Cancelled):
    await trio.lowlevel.checkpoint()

[oremanj]

So don't use check_takes_about (and don't mark this test as slow). Instead do something like:

try:
    await trio.lowlevel.checkpoint()
except trio.Cancelled:
    pytest.fail("shield didn't work")
inner.shield = False
with pytest.raises(trio.Cancelled):
    await trio.lowlevel.checkpoint()

[CoolCat467]

I think these changes are good, but I am curious about the root issue behind this. Are there use cases where it's not sufficient to set the shield to True slightly after it's initialized? And even in that case, couldn't you just create the cancel scope, set the shield attribute, and then enter it?

[agnesnatasya]

Are there use cases where it's not sufficient to set the shield to True slightly after it's initialized? And even in that case, couldn't you just create the cancel scope, set the shield attribute, and then enter it?

This is sufficient, and this change is just to have a one-liner syntax for this behavior.

[CoolCat467]

Looks good!

[A5rocks]

I was looking at whether this has been discussed before and found #147 where @njsmith thinks that making shield=True too ergonomic is an issue because people shouldn't shield much. On the other hand, flake8-async suggests people shield async statements in except (ASYNC120), libraries might have to use shields to make things safe for cancellation, and that issue seems to have gotten nowhere.

On the other other hand, the "library" I'm looking at (hypercorn's TCP send implementation) manually sets scope.shield = True even though shield=True is a kwarg for CancelScope(). @oremanj since you talked in that issue and are here already, what do you think?

(really this comment belongs in the related issue but... already written, what can you do.)

[oremanj]

Sorry for the delay, have been ill this week.

@A5rocks I would say that if we're trying to encourage use of shield+timeout over just shield-without-timeout -- which I think we are -- then we shouldn't make shield+timeout be cumbersome to write. Currently one must do either

with trio.CancelScope(shield=True) as scope:
    scope.deadline = trio.current_time() + 5
    <stuff>

or

with trio.move_on_after(5) as scope:
    scope.shield = True
    <stuff>

or

with trio.CancelScope(shield=True, deadline=trio.current_time() + 5):
    <stuff>

none of which exactly roll off the fingers. I think we should allow

with trio.move_on_after(5, shield=True):
    <stuff>

so people aren't pushed toward the more uncancellable-hang-prone

with trio.CancelScope(shield=True):
    <stuff>

If we don't want to make shielding visible in move_on_after and friends, then I think we should support trio.CancelScope(timeout=5, shield=True). But that then has an ambiguity associated with it (does the timeout start when the cancel scope is constructed or does it start when you enter the with block?) which seems like more interface-design trouble than it's worth.

Overall I think shielding is more important in moderately-complex-and-up programs than Trio's initial design gave it credit for.

[A5rocks]

Sounds good then!

[trio-bot]

Hey @agnesnatasya, it looks like that was the first time we merged one of your PRs! Thanks so much! 🎉 🎂

If you want to keep contributing, we'd love to have you. So, I just sent you an invitation to join the python-trio organization on Github! If you accept, then here's what will happen:

• Github will automatically subscribe you to notifications on all our repositories. (But you can unsubscribe again if you don't want the spam.)

• You'll be able to help us manage issues (add labels, close them, etc.)

• You'll be able to review and merge other people's pull requests

• You'll get a [member] badge next to your name when participating in the Trio repos, and you'll have the option of adding your name to our member's page and putting our icon on your Github profile (details)

If you want to read more, here's the relevant section in our contributing guide.

Alternatively, you're free to decline or ignore the invitation. You'll still be able to contribute as much or as little as you like, and I won't hassle you about joining again. But if you ever change your mind, just let us know and we'll send another invitation. We'd love to have you, but more importantly we want you to do whatever's best for you.

If you have any questions, well... I am just a humble Python script, so I probably can't help. But please do post a comment here, or in our chat, or on our forum, whatever's easiest, and someone will help you out!