Clarify eg-wrap contract + tighten `stdout` typing
Doc/type accuracy in `supervise_run_process()`: - the docstring claimed the rc!=0 `CalledProcessError` had "no nursery-eg-wrapped CPE to catch/`collapse_eg`", but it IS raised as a task into the parent `tn`, so under `trio>=0.33` it surfaces `ExceptionGroup`-wrapped like any `tn.start()`-task raise. Say so + point at `except*` / `collapse_eg()` (matching the inline rc-check comment). - widen `stdout: int` to `int|None` — the param accepts an explicit `None` (inherit) per its own docstring. - note that stdout is NOT captured, so the CPE note carries only stderr (a cmd logging errors to stdout should `relay_stdout=True`). Review: PR #465 (copilot-pull-request-reviewer) https://github.com/goodboy/tractor/pull/465#pullrequestreview-4548191641 (this patch was generated in some part by [`claude-code`][claude-code-gh]) [claude-code-gh]: https://github.com/anthropics/claude-codewkt/start_or_cancel_tests_474
parent
02cb789901
commit
6764251e6b
|
|
@ -160,7 +160,7 @@ async def supervise_run_process(
|
|||
# non-relay `stdout` override; defaults (via `_UNSET`) to
|
||||
# `DEVNULL` so we NEVER inherit (+ thus can't clobber) the
|
||||
# parent controlling-tty.
|
||||
stdout: int = _UNSET,
|
||||
stdout: int|None = _UNSET,
|
||||
|
||||
task_status: trio.TaskStatus[
|
||||
trio.Process
|
||||
|
|
@ -178,11 +178,14 @@ async def supervise_run_process(
|
|||
|
||||
- surfaces a rc!=0 `subprocess.CalledProcessError`
|
||||
DETERMINISTICALLY: we pass `check=False` to `trio` and
|
||||
do our OWN post-drain rc-check, (re)building + raising a
|
||||
BARE CPE (with a `.stderr` note) from this coro's body
|
||||
AFTER the child exits — so there's no nursery-eg-wrapped
|
||||
CPE to catch/`collapse_eg`, and the relay reader is never
|
||||
race-cancelled mid-drain.
|
||||
do our OWN post-drain rc-check, (re)building + raising the
|
||||
CPE (with a `.stderr` note) from this coro's body AFTER the
|
||||
child exits — so it's never wrapped by `trio.run_process`'s
|
||||
INTERNAL nursery nor race-cancelled mid-drain. It IS still
|
||||
raised as a task into the *parent* `tn`, so — like any
|
||||
`tn.start()`-task raise — it surfaces `ExceptionGroup`-
|
||||
wrapped under `trio>=0.33`; unwrap via `except*` or
|
||||
`trionics.collapse_eg()`.
|
||||
|
||||
- ALWAYS isolates the parent controlling-tty
|
||||
(`stdin=DEVNULL`, and `stdout=DEVNULL` unless
|
||||
|
|
@ -339,6 +342,10 @@ async def supervise_run_process(
|
|||
if stderr_accum is not None
|
||||
else b''
|
||||
)
|
||||
# NOTE: stdout is NOT captured (DEVNULL unless overridden)
|
||||
# so the CPE carries only (tail-bounded) stderr; a cmd that
|
||||
# logs failures to *stdout* should `relay_stdout=True` to
|
||||
# surface them in the note.
|
||||
cpe = subprocess.CalledProcessError(
|
||||
returncode=trio_proc.returncode,
|
||||
cmd=trio_proc.args,
|
||||
|
|
|
|||
Loading…
Reference in New Issue