Raise runtime error when submitting after executor shutdown

[jan-janssen]

Summary by CodeRabbit

• Bug Fixes
  • Executors now prevent scheduling new tasks after shutdown.
  • Attempting to submit tasks to a shut down executor raises an error.

[coderabbitai]

Walkthrough

This PR introduces a shutdown guard mechanism to the BaseExecutor class. An internal _is_active flag is initialized to True in the constructor and set to False during shutdown and __exit__ operations. The submit method now checks this flag to prevent scheduling new tasks after shutdown, with a corresponding test validating this behavior.

Changes

Cohort / File(s)	Summary
Executor shutdown guard implementation src/executorlib/executor/base.py	Adds _is_active internal flag initialized to True; submit guards against scheduling when flag is False; shutdown and __exit set flag to False after delegating to underlying task scheduler
Test validation tests/test_singlenodeexecutor_noblock.py	Adds new test method test_single_node_executor_exit to verify that RuntimeError is raised on task submission attempts after shutdown

Sequence Diagram

sequenceDiagram
    participant Client
    participant Executor
    participant TaskScheduler
    
    Client->>Executor: submit(task)
    alt _is_active == True
        Executor->>TaskScheduler: schedule task
        TaskScheduler-->>Executor: ✓
        Executor-->>Client: future
    else _is_active == False
        Executor-->>Client: ✗ RuntimeError
    end
    
    Client->>Executor: shutdown()
    Executor->>TaskScheduler: shutdown
    Executor->>Executor: _is_active = False
    TaskScheduler-->>Executor: ✓
    
    Client->>Executor: submit(task)
    Executor-->>Client: ✗ RuntimeError (guarded)

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Straightforward guard mechanism using a boolean flag
• Changes are localized to core executor logic and test
• Key areas for attention:
  • Verify _is_active is properly initialized in constructor
  • Confirm all shutdown paths (both shutdown() and __exit__()) set the flag to False
  • Check that submit() guard raises appropriate error type and message
  • Ensure test coverage captures both pre- and post-shutdown submission attempts

Poem

🐰 A guard stands at the executor's gate,
Preventing tasks when it's too late,
Once shutdown's called, _is_active falls,
No new submissions heed the calls—
Safe and sound, the executor rests! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The pull request title "Raise runtime error when submitting after executor shutdown" accurately describes the main change implemented in this changeset. The modifications to src/executorlib/executor/base.py introduce an _is_active flag that prevents task submission after shutdown, while the new test in tests/test_singlenodeexecutor_noblock.py validates this behavior by confirming a RuntimeError is raised on post-shutdown submissions. The title is concise, specific, and avoids vague language or noise, making it clear to developers reviewing the history what the primary intent of the change is.

✨ 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 runtimeerror

---

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.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

src/executorlib/executor/base.py (1)

103-108: Good shutdown guard implementation; consider unpacking syntax.

The logic correctly prevents submissions after shutdown and provides a clear error message.

Consider using the unpacking syntax as suggested by static analysis:

         if self._is_active:
             return self._task_scheduler.submit(
-                *([fn] + list(args)), resource_dict=resource_dict, **kwargs
+                fn, *args, resource_dict=resource_dict, **kwargs
             )

This is more idiomatic and avoids the list concatenation and unpacking overhead. Based on learnings

tests/test_singlenodeexecutor_noblock.py (1)

170-175: Good test coverage for explicit shutdown; consider testing context manager exit.

The test correctly validates that submissions are blocked after calling shutdown().

Consider adding a complementary test for the context manager exit path:

def test_single_node_executor_context_manager_exit(self):
    with SingleNodeExecutor(max_workers=2) as exe:
        self.assertEqual(exe.submit(sum, [1, 2, 3]).result(), 6)
    # After exiting context manager
    with self.assertRaises(RuntimeError):
        exe.submit(sum, [1, 2, 3])

This would verify that the shutdown guard also works when the executor exits via __exit__.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8957a38 and 1a993b1.

📒 Files selected for processing (2)

• src/executorlib/executor/base.py (4 hunks)
• tests/test_singlenodeexecutor_noblock.py (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (2)

tests/test_singlenodeexecutor_noblock.py (1)

src/executorlib/executor/base.py (4)

• max_workers (27-28)
• max_workers (31-32)
• submit (71-108)
• shutdown (110-126)

src/executorlib/executor/base.py (3)

src/executorlib/task_scheduler/base.py (1)

• submit (89-144)

src/executorlib/task_scheduler/interactive/blockallocation.py (1)

• submit (123-160)

src/executorlib/task_scheduler/interactive/dependency.py (1)

• submit (113-153)

🪛 Ruff (0.14.1)

src/executorlib/executor/base.py

105-105: Consider [fn, *list(args)] instead of concatenation

Replace with [fn, *list(args)]

(RUF005)

---

108-108: Avoid specifying long messages outside the exception class

(TRY003)

🔇 Additional comments (3)

src/executorlib/executor/base.py (3)

24-24: LGTM! Clean initialization of the shutdown guard.

The _is_active flag is properly initialized to allow submissions until shutdown.

---

125-126: LGTM! Proper flag update after shutdown.

The _is_active flag is correctly set to False after the task scheduler shutdown completes.

---

150-151: LGTM! Context manager exit properly handled.

The _is_active flag is correctly set to False after the context manager exit, ensuring the shutdown guard works with with statements.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.26%. Comparing base (7aa3d51) to head (1a993b1).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #851      +/-   ##
==========================================
+ Coverage   93.24%   93.26%   +0.01%     
==========================================
  Files          38       38              
  Lines        1791     1796       +5     
==========================================
+ Hits         1670     1675       +5     
  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.