fix: forward cancellation token to toolkit installer subprocesses#409
fix: forward cancellation token to toolkit installer subprocesses#409sLightlyDev wants to merge 1 commit into
Conversation
The pip/venv exec calls in the Deepnote toolkit installer were started without the CancellationToken, so cancellation was only checked between calls. Cancelling during a multi-minute pip install did nothing until the install finished, leaving the Stop button unresponsive. Pass the token into every processService.exec call (and thread it through isToolkitInstalled) so cancelling now terminates the running subprocess immediately.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #409 +/- ##
===========================
===========================
🚀 New features to boost your workflow:
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds cancellation-token propagation to all subprocess executions in the deepnote-toolkit installer. The Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Problem
The pip/venv
processService.execcalls in the Deepnote toolkit installer were started without theCancellationToken. Cancellation was only checked between calls, so cancelling during a multi-minutepip installdid nothing until the install finished — the Stop button appeared dead.Fix
Pass the token into every
execcall indeepnoteToolkitInstaller.node.ts(and thread it throughisToolkitInstalled). The process layer already wirestoken.onCancellationRequestedto kill the subprocess, so cancelling now terminates the running install immediately.Testing
exectsccleanOpened as draft pending review.
Summary by CodeRabbit
Bug Fixes
Tests