Skip to content

Test restart_limit #813

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.

Sign up for GitHub

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

Merged
jan-janssen merged 1 commit into from
Sep 8, 2025
Merged

Test restart_limit #813

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion tests/test_singlenodeexecutor_noblock.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ def test_single_node_executor(self):

def test_single_node_executor_block_allocation(self):
with self.assertRaises(ExecutorlibSocketError):
with SingleNodeExecutor(max_workers=2, block_allocation=True) as exe:
with SingleNodeExecutor(max_workers=2, block_allocation=True, resource_dict={"restart_limit": 2}) as exe:
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Good addition; please verify the key propagates end-to-end.

Passing resource_dict={"restart_limit": 2} looks correct and aligns with the SingleNodeExecutor docstring. However, default_resource_dict in executorlib/executor/single.py doesn’t include restart_limit, so this key must be explicitly handled downstream (e.g., in create_single_node_executor). Please verify that restart_limit is actually consumed and not silently ignored in both block_allocation=True/False paths.

Run this to confirm usage coverage and call paths:

🏁 Script executed:

#!/bin/bash
# Where is restart_limit referenced?
rg -nC3 -P '\brestart_limit\b'

# Where is create_single_node_executor defined and how does it use resource_dict?
rg -nC5 -P 'def\s+create_single_node_executor\b'
rg -nC5 -P 'create_single_node_executor\('

# Check that worker (re)spawn logic reads restart_limit
rg -nC3 -P '(restart|respawn|relaunch).*restart_limit|restart_limit.*(restart|respawn|relaunch)'

Length of output: 17219

🏁 Script executed:

#!/bin/bash
# Inspect merging of default_resource_dict and resource_dict
sed -n '140,200p' executorlib/executor/single.py

# Inspect create_single_node_executor implementation
sed -n '360,450p' executorlib/executor/single.py

# Check for any direct usage of restart_limit in this file
rg -n 'restart_limit' -C5 executorlib/executor/single.py

Length of output: 9024

restart_limit ignored in executor pipeline
The restart_limit key from resource_dict isn’t read anywhere in create_single_node_executor (executorlib/executor/single.py:create_single_node_executor) nor by the spawned schedulers, so setting resource_dict={"restart_limit":2} has no effect. Add explicit handling—e.g. extend create_single_node_executor to pop restart_limit and pass it into the OneProcessTaskScheduler/BlockAllocationTaskScheduler or their spawners.

🤖 Prompt for AI Agents 
In tests/test_singlenodeexecutor_noblock.py around line 160, the test sets
resource_dict={"restart_limit": 2} but this key is ignored by
create_single_node_executor and the spawned schedulers; update
executorlib/executor/single.py:create_single_node_executor to explicitly extract
(pop) "restart_limit" from resource_dict and forward it as a parameter to the
scheduler factory or spawner
(OneProcessTaskScheduler/BlockAllocationTaskScheduler) when
constructing/starting schedulers; ensure scheduler constructors/spawners accept
a restart_limit argument (add parameter and store/use it) and propagate it into
the process/task restart logic, and update any call sites that create schedulers
to pass the extracted restart_limit so the test's resource_dict takes effect.

f = exe.submit(exit_funct)
print(f.result())

Expand Down
Loading