fix(followups): F4a/F4c - the runner's wait knobs follow the integer rule; MAM_WAIT_GRACE_S documented
MAM_WAIT_GRACE_S silently fell back to 300 on anything but bare digits
(" 30 " too), MAM_CHILD_WAIT_S went through float() (abc raised ValueError,
0.5 and 1e3 were taken) and MAM_JOB_TREE_MAX through int() (abc and an
empty value raised). All three now go through runner._int_knob, which is
_positive_int's rule (PR #43 review grok): decimal, blanks around allowed,
an invalid value warns once and the default is used. Minimums:
MAM_WAIT_GRACE_S >= 0 (wait exactly the budget), MAM_CHILD_WAIT_S >= 0
(look once; test_issue34 uses 0), MAM_JOB_TREE_MAX >= 1 (a cap of 0 calls
every tree "over"; not waiting is MAM_CHILD_WAIT_S=0).
The loop SKILL.md now names MAM_WAIT_GRACE_S in the budget row and the
sub-job paragraph (PR #41 M6), with the knobs' integer rules.
Tests: RUNNER_INT_KNOBS in test_issue26_ints.py holds the minimums; the
real wait_for_descendants is driven per knob, and a static check ties the
call-site minimums to the table (15 of the new cases fail at 0051fd6; the
14 that pass are valid values whose result did not change).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HCGpJXhSZasJLU38ksDghe
This commit is contained in:
1 parent
71ea8095c5
commit
8b046390be
3 files changed
+92
-8
No files matched your search
@@ -53,6 +53,15 @@ INT_KNOBS = {
|
||||
"--ack-timeout": (1, None),
|
||||
}
|
||||
|
||||
# The loop runner's own integer knobs (follow-up F4a): read by runner._int_knob
|
||||
# (_positive_int's rule) with these minimums; an invalid value warns once and
|
||||
# the default is used - never an exception. knob -> (minimum, default)
|
||||
RUNNER_INT_KNOBS = {
|
||||
"MAM_WAIT_GRACE_S": (0, 300), # 0: wait exactly the job budget
|
||||
"MAM_CHILD_WAIT_S": (0, None), # 0: look at the sub-jobs once; default: budget + grace
|
||||
"MAM_JOB_TREE_MAX": (1, 50), # 0 would refuse every tree as "over"; not waiting is MAM_CHILD_WAIT_S=0
|
||||
}
|
||||
|
||||
# The last two are longer than Python's int() digit limit (4300): the rule must
|
||||
# bound the significant digits before converting (review R3).
|
||||
INPUTS = ["30", "0700", "0900", " 30 ", "0", "-5", "abc", "", "1e3", "9999999999", "000000000030",
|
||||
@@ -132,6 +141,21 @@ def _runner(home: Path, extra_env=None, *argv) -> subprocess.CompletedProcess:
|
||||
env=_base_env(home, extra_env), cwd=str(home), timeout=60)
|
||||
|
||||
|
||||
def _runner_wait(home: Path, extra_env=None, kids: int = 0) -> tuple:
|
||||
"""The real runner waiting on a sub-job tree of `kids` open jobs (budget 1 s):
|
||||
({grace, ok}, everything it printed)."""
|
||||
code = ("import json, sys; from lib_py.loop import runner as lr; "
|
||||
"r = lr.LoopRunner(['--creator', 'c', '--task', 't', '--job-timeout', '1']); r.parse_args(); "
|
||||
"lr.job_registry.descendants_of = lambda root, d: [{'job_id': 'k%d' % i, 'status': 'running'} "
|
||||
"for i in range(int(sys.argv[1]))]; "
|
||||
"ok = r.wait_for_descendants('root', interval=0.1); "
|
||||
"print(json.dumps({'grace': r.job_wait_s - r.job_budget, 'ok': ok}))")
|
||||
res = subprocess.run([sys.executable, "-c", code, str(kids)], capture_output=True, text=True,
|
||||
env=_base_env(home, {k: v for k, v in (extra_env or {}).items()}), cwd=str(home), timeout=60)
|
||||
assert res.returncode == 0 and "Traceback" not in res.stderr, res.stderr[-800:]
|
||||
return json.loads(res.stdout.strip().splitlines()[-1]), res.stdout + res.stderr
|
||||
|
||||
|
||||
def _bash(snippet: str, home: Path, extra_env=None) -> subprocess.CompletedProcess:
|
||||
return subprocess.run(["bash", "-c", f'source "{LIB_SH}" >/dev/null 2>&1; {snippet}'], capture_output=True,
|
||||
text=True, env=_base_env(home, extra_env), cwd=str(home), timeout=60)
|
||||
@@ -214,6 +238,50 @@ def test_b1_sks_knobs_keep_zero_and_read_decimal(tmp_path, knob):
|
||||
assert ("ignored" in res.stderr) is warns, (value, res.stderr)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("value", INPUTS, ids=_id)
|
||||
def test_b1_runner_wait_grace_is_decimal_or_the_default(tmp_path, value):
|
||||
minimum, default = RUNNER_INT_KNOBS["MAM_WAIT_GRACE_S"]
|
||||
want = _expected(value, minimum)
|
||||
got, out = _runner_wait(tmp_path, {"MAM_WAIT_GRACE_S": value})
|
||||
assert got["grace"] == (default if want is None else want), out[-500:]
|
||||
assert ("WARNING: MAM_WAIT_GRACE_S=" in out and "ignored" in out) is (want is None and value != ""), out[-500:]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("value,waited,warns", [("0", "0s (MAM_CHILD_WAIT_S)", False),
|
||||
(" 0 ", "0s (MAM_CHILD_WAIT_S)", False),
|
||||
("01", "1s (MAM_CHILD_WAIT_S)", False),
|
||||
("abc", "1s (MAM_CHILD_WAIT_S default", True),
|
||||
("-5", "1s (MAM_CHILD_WAIT_S default", True),
|
||||
("0.5", "1s (MAM_CHILD_WAIT_S default", True),
|
||||
("9999999999", "1s (MAM_CHILD_WAIT_S default", True),
|
||||
("", "1s (MAM_CHILD_WAIT_S default", False)])
|
||||
def test_b1_runner_child_wait_is_decimal_or_the_default(tmp_path, value, waited, warns):
|
||||
"""MAM_CHILD_WAIT_S=abc raised ValueError; the default is budget + grace (1 + 0 here)."""
|
||||
got, out = _runner_wait(tmp_path, {"MAM_CHILD_WAIT_S": value, "MAM_WAIT_GRACE_S": "0"}, kids=1)
|
||||
assert got["ok"] is False and f"ended after {waited}" in out, out[-500:]
|
||||
assert ("WARNING: MAM_CHILD_WAIT_S=" in out and "ignored" in out) is warns, out[-500:]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("value,over,warns", [("2", True, False), ("03", False, False), (" 2 ", True, False),
|
||||
("0", False, True), ("abc", False, True), ("-1", False, True),
|
||||
("", False, False)])
|
||||
def test_b1_runner_job_tree_max_is_decimal_or_the_default(tmp_path, value, over, warns):
|
||||
"""Three open sub-jobs against the cap; an invalid cap is 50 with a warning."""
|
||||
got, out = _runner_wait(tmp_path, {"MAM_JOB_TREE_MAX": value, "MAM_CHILD_WAIT_S": "0"}, kids=3)
|
||||
assert got["ok"] is False
|
||||
assert ("over MAM_JOB_TREE_MAX=" in out) is over, out[-500:]
|
||||
assert ("ended after 0s" in out) is (not over), out[-500:]
|
||||
assert ("WARNING: MAM_JOB_TREE_MAX=" in out and "ignored" in out) is warns, out[-500:]
|
||||
|
||||
|
||||
def test_b5_runner_knobs_use_the_table_minimum():
|
||||
src = (SKILLS / "lib_py" / "loop" / "runner.py").read_text()
|
||||
calls = dict(re.findall(r'_int_knob\("([A-Z_]+)", (\d+)\)', src))
|
||||
assert {k: int(v) for k, v in calls.items()} == {k: m for k, (m, _) in RUNNER_INT_KNOBS.items()}
|
||||
for name in RUNNER_INT_KNOBS:
|
||||
assert f'os.environ.get("{name}"' not in src, f"{name} is read raw"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# B2 - flags
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
Reference in new issue
Block a user