Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Delay deciding which cancel scope a Cancelled exception belongs to #901

Merged
merged 3 commits into from Feb 5, 2019
Merged

Delay deciding which cancel scope a Cancelled exception belongs to #901

merged 3 commits into from Feb 5, 2019

Conversation

oremanj
Copy link
Member

@oremanj oremanj commented Feb 4, 2019

Fixes #860.

@oremanj oremanj requested a review from njsmith February 4, 2019 01:22
@codecov
Copy link

codecov bot commented Feb 4, 2019

Codecov Report

Merging #901 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #901      +/-   ##
==========================================
- Coverage   99.53%   99.53%   -0.01%     
==========================================
  Files         102      102              
  Lines       12411    12397      -14     
  Branches      916      915       -1     
==========================================
- Hits        12353    12339      -14     
  Misses         36       36              
  Partials       22       22
Impacted Files Coverage Δ
trio/_core/_exceptions.py 100% <ø> (ø) ⬆️
trio/_core/tests/test_run.py 100% <100%> (ø) ⬆️
trio/tests/test_highlevel_open_tcp_stream.py 100% <100%> (ø) ⬆️
trio/_core/_run.py 99.7% <100%> (-0.01%) ⬇️

@codecov
Copy link

codecov bot commented Feb 4, 2019 •
edited

Codecov Report

Merging #901 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #901      +/-   ##
==========================================
- Coverage   99.53%   99.53%   -0.01%     
==========================================
  Files         102      102              
  Lines       12411    12409       -2     
  Branches      916      915       -1     
==========================================
- Hits        12353    12351       -2     
  Misses         36       36              
  Partials       22       22
Impacted Files Coverage Δ
trio/_core/_exceptions.py 100% <ø> (ø) ⬆️
trio/_core/tests/test_run.py 100% <100%> (ø) ⬆️
trio/tests/test_highlevel_open_tcp_stream.py 100% <100%> (ø) ⬆️
trio/_core/_run.py 99.7% <100%> (-0.01%) ⬇️
trio/_core/tests/test_multierror.py 100% <0%> (ø) ⬆️

njsmith
njsmith reviewed Feb 4, 2019
View reviewed changes
Copy link
Member

@njsmith njsmith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, this is simpler than I was expecting :-)

trio/_core/_exceptions.py Outdated
crash your program. Use :meth:`cancel_scope.cancel()
<trio.CancelScope.cancel>` instead, so Trio can maintain the
invariant that a :exc:`Cancelled` exception only gets raised when
there's a cancelled cancel scope that's intended to catch it.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably more details on the internals than we really want people thinking about. How about simplifying down to just: "You cannot raise Cancelled yourself. Attempting to do so will produce a RuntimeError. Use cancel_scope.cancel instead."

@oremanj
Copy link
Member Author

oremanj commented Feb 4, 2019

The 3.8-dev failure in Travis is unrelated, so I think this is ready.

@njsmith
Copy link
Member

njsmith commented Feb 4, 2019

LGTM. That travis failure is annoying... I just filed a bug on CPython (bpo-35894), so hopefully it will be fixed soon. In the mean time though our CI is broken. I guess we need to temporarily disable the 3.8-dev test (or mark it "allowed to fail") until that's fixed?

@njsmith
Copy link
Member

njsmith commented Feb 5, 2019

Closing/opening to tickle CI, now that #905 is merged.

@njsmith njsmith closed this Feb 5, 2019
@njsmith njsmith reopened this Feb 5, 2019
@oremanj
Copy link
Member Author

oremanj commented Feb 5, 2019

This time py37_latest_ubuntu on Azure seems to have timed out before it ran any of our code (reeeally slow downloading packages). I... guess I'll try again?

@oremanj oremanj closed this Feb 5, 2019
@oremanj oremanj reopened this Feb 5, 2019
@oremanj oremanj merged commit 3aaa6ba into python-trio:master Feb 5, 2019
oremanj added a commit to oremanj/trio that referenced this pull request Feb 5, 2019
@oremanj
Cancel a cancel scope upon entry if its deadline is in the past
75725a8 
Fixes python-trio#320. Now that python-trio#901 is implemented, the concerns discussed in python-trio#835 don't apply.
@oremanj oremanj mentioned this pull request Feb 5, 2019
Merged
@oremanj oremanj mentioned this pull request Mar 1, 2019
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@njsmith njsmith njsmith left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@oremanj @njsmith