Skip to content

Commit c797bcb

Browse files
committed
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) #462 (review) (this patch was generated in some part by [`claude-code`][claude-code-gh]) [claude-code-gh]: https://github.com/anthropics/claude-code
1 parent 7b518fe commit c797bcb

5 files changed

Lines changed: 17 additions & 16 deletions

File tree

examples/debugging/sync_bp.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
# disable `pbdp` prompt colors
1313
# for prompt matching in test.
1414
def disable_pdbp_color():
15-
if os.environ['PYTHON_COLORS'] == '0':
15+
if os.environ.get('PYTHON_COLORS') == '0':
1616
from tractor.devx.debug import _repl
1717
_repl.TractorConfig.use_pygments = False
1818

tests/trionics/test_patches.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ def test_wakeup_socketpair_drain_eof_patch_works():
5959
# First call MUST return True; idempotent guard
6060
# prevents False on subsequent calls within the
6161
# same process.
62-
assert applied is True or applied is False # idempotent
62+
assert isinstance(applied, bool) # idempotent (order-dependent value)
6363

6464
# Cap wall-clock at 2s; SIGALRM raises in main
6565
# thread which interrupts the C-level recv loop

tractor/devx/_stackscope.py

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -71,17 +71,18 @@ def dump_task_tree(
7171
Do a classic `stackscope.extract()` task-tree dump to console at
7272
`.devx()` level.
7373
74-
Also unconditionally tee the rendered tree to two
75-
capture-bypassing sinks so SIGUSR1 dumps remain visible
76-
when the parent process has captured stdio (e.g. pytest's
77-
default `--capture=fd`):
78-
79-
- `/tmp/tractor-stackscope-<pid>.log` (append-mode, always
80-
written) — guaranteed-readable artifact even under CI
74+
When `write_file`/`write_tty` are set, ALSO tee the rendered
75+
tree to capture-bypassing sinks so SIGUSR1 dumps remain
76+
visible when the parent process has captured stdio (e.g.
77+
pytest's default `--capture=fd`); the SIGUSR1 handler passes
78+
`write_file=True` for exactly this reason:
79+
80+
- `write_file` -> `/tmp/tractor-stackscope-<pid>.log`
81+
(append-mode) — guaranteed-readable artifact even under CI
8182
/ `nohup` / no-tty conditions. `tail -f` to follow.
82-
- `/dev/tty` if a controlling terminal is attached —
83-
best-effort, ignored if the device is missing or write
84-
fails. pytest never captures the tty.
83+
- `write_tty` -> `/dev/tty` if a controlling terminal is
84+
attached — best-effort, ignored if the device is missing
85+
or write fails. pytest never captures the tty.
8586
8687
'''
8788
import os

tractor/ipc/_linux.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
try:
2525
import cffi
2626
except ImportError as ie:
27-
if sys.version_info < (3, 14):
27+
if sys.version_info >= (3, 14):
2828
ie.add_note(
2929
f'The `cffi` pkg has no 3.14 support yet.\n'
3030
)

tractor/runtime/_portal.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -340,9 +340,9 @@ async def cancel_actor(
340340
raise_on_timeout
341341
):
342342
raise ActorTooSlowError(
343-
f'Peer {peer_id} did not ack `Actor.cancel()`'
344-
f'-RPC within bounded wait of '
345-
f'{cancel_timeout!r}s'
343+
f'Peer {peer_id} did not ack its '
344+
f'`Actor.cancel()` RPC within bounded wait '
345+
f'of {cancel_timeout!r}s'
346346
)
347347

348348
# legacy fire-and-forget path: log + return False so

0 commit comments

Comments
 (0)