avoid holding a reference to exception and value in to_thread_run_sync #3229
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3229 +/- ##
===============================================
Coverage 100.00000% 100.00000%
===============================================
Files 124 124
Lines 18779 18837 +58
Branches 1269 1270 +1
===============================================
+ Hits 18779 18837 +58
🚀 New features to boost your workflow:
|
graingert
force-pushed
the
fix-to-thread-keeping-hold-of-outcome
branch
from
98c1100 to
fc2edc7
Compare
There was a problem hiding this comment.
This seems like just reimplementing outcome. If you need to copy, why not instantiate outcome.Value/Error directly? Only detail that's different is unwrap doesn't clear values, but we could just inline the implementation into the unwrap location? Or add unwrap_clear() or something.
oh yeah that's the plan, I want to get a proof of concept working first |
graingert
changed the title
| @@ -46,6 +46,7 @@ python -m uv pip install build | |||
| python -m build | |||
| wheel_package=$(ls dist/*.whl) | |||
| python -m uv pip install "trio @ $wheel_package" -c test-requirements.txt | |||
| python -m uv pip install https://github.com/python-trio/outcome/archive/e0f317813a499f1a3629b37c3b8caed72825d9c0.zip | |||
There was a problem hiding this comment.
No I've not got the change landed in Outcome
There was a problem hiding this comment.
We're going to leave a pinned commit in ci.sh? This seems sketchy.
There was a problem hiding this comment.
No, once everything's decided on and merged in I'll update the pyproject.toml and remove this
There was a problem hiding this comment.
If python-trio/outcome#45 (comment) is true then I'd at least like a PR that removes dependence on those, even if it doesn't come with tested guarantees. That way any sort of outcome 2.0 release isn't as annoying.
But I also see this PR doesn't change any .value or .error so I assume we already conform with clear-on-unwrap semantics?
No description provided.