-
-
Notifications
You must be signed in to change notification settings - Fork 14
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
Avoid creating a reference cycle when calling Error.unwrap #29
Conversation
When put together, this change + python-trio/outcome#29 should fix python-trio#1770
103eafa
to
5977c62
Compare
This avoids invoking the cycle collector as often; see python-trio/trio#1770 for more details.
5977c62
to
85502c5
Compare
Codecov Report
@@ Coverage Diff @@
## master #29 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 6 6
Lines 213 227 +14
Branches 11 11
=========================================
+ Hits 213 227 +14
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what do you think about adding a test case that checks the cycle count created by a sleep(0)
(probably for CPython only)? Set the limit at or around the current observed value. Then we can tighten this as improvements are added, and have a chance at recognizing regressions early.
(moving comment to trio PR)
Co-authored-by: Dave Hirschfeld <dave.hirschfeld@gmail.com>
This is going to need admin permissions to do the actual merge, because I just switched the repo from travis-ci.org to travis-ci.com and the branch protection rules are temporarily messed up. But I know @belm0 uses "approve" to mean "please go ahead and self merge", so I guess I'll take that as permission to self-merge :-) |
worth a newsfragment? |
When put together, this change + python-trio/outcome#29 should fix python-trio#1770
This avoids invoking the cycle collector as often; see
python-trio/trio#1770
for more details.