Address Copilot review nits on PR #462
All 5 flagged items were valid (4 real bugs + 1 dead assert),
- fix an inverted `sys.version_info < (3, 14)` guard in
`ipc._linux` — the "`cffi` has no 3.14 support" import note now
fires on 3.14+ (where it applies) instead of on older pys.
- use `os.environ.get('PYTHON_COLORS')` in the `sync_bp` example
so it doesn't `KeyError` when run outside the test harness.
- correct `dump_task_tree()`'s docstring: the `/tmp` + `/dev/tty`
tee is gated on `write_file`/`write_tty`, not "unconditional".
- tidy the `ActorTooSlowError` message spacing in `cancel_actor`.
- replace a tautological `applied is True or applied is False` in
`test_patches` with `isinstance(applied, bool)` (the value is
order-dependent across the module).
Review: PR #462 (copilot-pull-request-reviewer)
https://github.com/goodboy/tractor/pull/462#pullrequestreview-4527179852
(this patch was generated in some part by [`claude-code`][claude-code-gh])
[claude-code-gh]: https://github.com/anthropics/claude-code
trionics_start_or_cancel
parent
7b518fe4e1
commit
c797bcb783
|
|
@ -12,7 +12,7 @@ import tractor
|
||||||
# disable `pbdp` prompt colors
|
# disable `pbdp` prompt colors
|
||||||
# for prompt matching in test.
|
# for prompt matching in test.
|
||||||
def disable_pdbp_color():
|
def disable_pdbp_color():
|
||||||
if os.environ['PYTHON_COLORS'] == '0':
|
if os.environ.get('PYTHON_COLORS') == '0':
|
||||||
from tractor.devx.debug import _repl
|
from tractor.devx.debug import _repl
|
||||||
_repl.TractorConfig.use_pygments = False
|
_repl.TractorConfig.use_pygments = False
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -59,7 +59,7 @@ def test_wakeup_socketpair_drain_eof_patch_works():
|
||||||
# First call MUST return True; idempotent guard
|
# First call MUST return True; idempotent guard
|
||||||
# prevents False on subsequent calls within the
|
# prevents False on subsequent calls within the
|
||||||
# same process.
|
# same process.
|
||||||
assert applied is True or applied is False # idempotent
|
assert isinstance(applied, bool) # idempotent (order-dependent value)
|
||||||
|
|
||||||
# Cap wall-clock at 2s; SIGALRM raises in main
|
# Cap wall-clock at 2s; SIGALRM raises in main
|
||||||
# thread which interrupts the C-level recv loop
|
# thread which interrupts the C-level recv loop
|
||||||
|
|
|
||||||
|
|
@ -71,17 +71,18 @@ def dump_task_tree(
|
||||||
Do a classic `stackscope.extract()` task-tree dump to console at
|
Do a classic `stackscope.extract()` task-tree dump to console at
|
||||||
`.devx()` level.
|
`.devx()` level.
|
||||||
|
|
||||||
Also unconditionally tee the rendered tree to two
|
When `write_file`/`write_tty` are set, ALSO tee the rendered
|
||||||
capture-bypassing sinks so SIGUSR1 dumps remain visible
|
tree to capture-bypassing sinks so SIGUSR1 dumps remain
|
||||||
when the parent process has captured stdio (e.g. pytest's
|
visible when the parent process has captured stdio (e.g.
|
||||||
default `--capture=fd`):
|
pytest's default `--capture=fd`); the SIGUSR1 handler passes
|
||||||
|
`write_file=True` for exactly this reason:
|
||||||
|
|
||||||
- `/tmp/tractor-stackscope-<pid>.log` (append-mode, always
|
- `write_file` -> `/tmp/tractor-stackscope-<pid>.log`
|
||||||
written) — guaranteed-readable artifact even under CI
|
(append-mode) — guaranteed-readable artifact even under CI
|
||||||
/ `nohup` / no-tty conditions. `tail -f` to follow.
|
/ `nohup` / no-tty conditions. `tail -f` to follow.
|
||||||
- `/dev/tty` if a controlling terminal is attached —
|
- `write_tty` -> `/dev/tty` if a controlling terminal is
|
||||||
best-effort, ignored if the device is missing or write
|
attached — best-effort, ignored if the device is missing
|
||||||
fails. pytest never captures the tty.
|
or write fails. pytest never captures the tty.
|
||||||
|
|
||||||
'''
|
'''
|
||||||
import os
|
import os
|
||||||
|
|
|
||||||
|
|
@ -24,7 +24,7 @@ import sys
|
||||||
try:
|
try:
|
||||||
import cffi
|
import cffi
|
||||||
except ImportError as ie:
|
except ImportError as ie:
|
||||||
if sys.version_info < (3, 14):
|
if sys.version_info >= (3, 14):
|
||||||
ie.add_note(
|
ie.add_note(
|
||||||
f'The `cffi` pkg has no 3.14 support yet.\n'
|
f'The `cffi` pkg has no 3.14 support yet.\n'
|
||||||
)
|
)
|
||||||
|
|
|
||||||
|
|
@ -340,9 +340,9 @@ class Portal:
|
||||||
raise_on_timeout
|
raise_on_timeout
|
||||||
):
|
):
|
||||||
raise ActorTooSlowError(
|
raise ActorTooSlowError(
|
||||||
f'Peer {peer_id} did not ack `Actor.cancel()`'
|
f'Peer {peer_id} did not ack its '
|
||||||
f'-RPC within bounded wait of '
|
f'`Actor.cancel()` RPC within bounded wait '
|
||||||
f'{cancel_timeout!r}s'
|
f'of {cancel_timeout!r}s'
|
||||||
)
|
)
|
||||||
|
|
||||||
# legacy fire-and-forget path: log + return False so
|
# legacy fire-and-forget path: log + return False so
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue