Feature: Cancel processes of cancelled futures

[jan-janssen]

Summary by CodeRabbit

• Refactor
  • Enhanced task cancellation detection in the scheduler with improved resource cleanup and termination procedures for cancelled tasks, ensuring more robust task execution workflows.

[coderabbitai]

📝 Walkthrough

Walkthrough

The _refresh_memory_dict function in the task scheduler's shared module was enhanced to accept additional contextual parameters (process_dict, terminate_function, pysqa_config_directory, backend) and now detects cancelled tasks to trigger termination actions accordingly.

Changes

Cohort / File(s)	Summary
Task Scheduler Cancellation Logic src/executorlib/task_scheduler/file/shared.py	Expanded _refresh_memory_dict signature to accept process_dict and optional termination parameters. Added logic to identify cancelled tasks and invoke _cancel_processes with appropriate context. Updated all call sites to pass new parameters through the execution flow.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A scheduler's heart grows wise and keen,
With cancellations caught between,
The processes now dance with grace,
As threads of context find their place,
Hopping forward, code so clean! 🌿

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Feature: Cancel processes of cancelled futures' directly reflects the main change: implementing cancellation of processes associated with cancelled futures.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Merge Conflict Detection	✅ Passed	✅ No merge conflicts detected when merging into main

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch cancel_completed_processes

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.62%. Comparing base (aa0f602) to head (d0b0bf3).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #903   +/-   ##
=======================================
  Coverage   93.61%   93.62%           
=======================================
  Files          38       38           
  Lines        1895     1897    +2     
=======================================
+ Hits         1774     1776    +2     
  Misses        121      121           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/executorlib/task_scheduler/file/shared.py (1)

292-309: ⚠️ Potential issue | 🟡 Minor

Cancelled task entries leak in process_dict.

After terminating cancelled processes, those keys are removed from memory_dict (line 308 filters out done() futures), but the corresponding entries remain in process_dict. This means:

1. Stale references accumulate for the lifetime of the executor.
2. On shutdown (line 102-107), _cancel_processes will attempt to re-terminate already-terminated processes.

Consider cleaning up process_dict alongside memory_dict:

Proposed fix

     cancelled_lst = [
         key for key, value in memory_dict.items() if value.done() and value.cancelled()
     ]
     _cancel_processes(
         process_dict={k: v for k, v in process_dict.items() if k in cancelled_lst},
         terminate_function=terminate_function,
         pysqa_config_directory=pysqa_config_directory,
         backend=backend,
     )
+    for key in cancelled_lst:
+        process_dict.pop(key, None)
     return {

🧹 Nitpick comments (1)

src/executorlib/task_scheduler/file/shared.py (1)

292-293: Nit: value.done() is redundant when combined with value.cancelled().

For concurrent.futures.Future, cancelled() implies done(). The value.done() guard can be dropped without changing behavior.

Proposed simplification

     cancelled_lst = [
-        key for key, value in memory_dict.items() if value.done() and value.cancelled()
+        key for key, value in memory_dict.items() if value.cancelled()
     ]