Skip to content

Commit c979ade

Browse files
prepare-sync: precise warn text + spec-derived test bodies
PR-C.3 review fixup. - observer.py: rewrite the awaitable-from-prepare_sync warning. The old text claimed "did NOT run", but a user returning an ``asyncio.Task`` / ``Future`` may have work in flight on the loop — just not awaited at the prepare_sync call site. The contract violation is "no guarantee the prep completes before the node body," not "definitely doesn't run." Reworded to that shape and included ``type(result).__name__`` so the user can see which awaitable they returned at a glance. - tests/conformance/test_observability.py: sub-case 1's driver hardcoded the YAML message bodies ("node a executing" / "node b executing") for record filtering and lookup, even though it had already read ``emits_log.message`` from the spec to drive the node body. That duplicated spec data and made the test brittle to fixture wording changes. Derive a ``node_emit_messages`` map from ``nodes_spec`` up front; use the values for both record filtering and ``by_body`` indexing. Sub-case 2 already worked this way (uses ``outer_emit`` / ``inner_emit`` derived from spec); sub-case 1 now matches.
1 parent 2746090 commit c979ade

2 files changed

Lines changed: 22 additions & 10 deletions

File tree

src/openarmature/graph/observer.py

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -435,10 +435,12 @@ def _dispatch(context: _InvocationContext, event: NodeEvent) -> None:
435435
stacklevel=2,
436436
)
437437
warnings.warn(
438-
"observer prepare_sync returned an awaitable; "
439-
"prepare_sync MUST be sync (define as `def`, not "
440-
"`async def`). The synchronous prep work did NOT "
441-
"run; log correlation will miss this node's span.",
438+
f"observer prepare_sync returned an awaitable "
439+
f"({type(result).__name__}); prepare_sync MUST be sync "
440+
f"(define as `def`, not `async def`). The returned "
441+
f"awaitable will not be awaited and is NOT guaranteed "
442+
f"to complete before the node body starts; log "
443+
f"correlation may miss this node's span.",
442444
stacklevel=2,
443445
)
444446
context.queue.put_nowait(_QueuedItem(event=event, observers=observers))

tests/conformance/test_observability.py

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1515,6 +1515,13 @@ async def _run_fixture_010_nested_trace(case: Mapping[str, Any]) -> None:
15151515

15161516
nodes_spec = cast("dict[str, Any]", case["nodes"])
15171517
correlation_id = cast("str", case["caller_correlation_id"])
1518+
# Spec YAML is the single source of truth for the log bodies; derive
1519+
# them up front rather than hard-coding so a fixture rename doesn't
1520+
# silently break the driver's record filtering.
1521+
node_emit_messages: dict[str, str] = {
1522+
name: cast("str", cast("dict[str, Any]", nodes_spec[name])["emits_log"]["message"])
1523+
for name in nodes_spec
1524+
}
15181525

15191526
class _S(State):
15201527
x: int = 0
@@ -1554,18 +1561,21 @@ async def body(_s: _S) -> dict[str, Any]:
15541561
log_provider.force_flush()
15551562

15561563
records = log_exporter.get_finished_logs()
1557-
# Filter to OUR test logger so concurrent test setup noise
1558-
# doesn't contaminate the assertions.
1559-
ours = [r for r in records if str(r.log_record.body) in {"node a executing", "node b executing"}]
1564+
# Filter to OUR test loggers so concurrent test setup noise
1565+
# doesn't contaminate the assertions. Expected message set
1566+
# comes from the spec YAML, not hard-coded strings.
1567+
expected_messages = set(node_emit_messages.values())
1568+
ours = [r for r in records if str(r.log_record.body) in expected_messages]
15601569
assert len(ours) == 2, (
15611570
f"expected 2 log records (one per node body); got {len(ours)}: "
15621571
f"{[str(r.log_record.body) for r in ours]}"
15631572
)
15641573

1565-
# Group by body for predictable lookup.
1574+
# Group by body for predictable lookup, indexing by the spec's
1575+
# emit-message values.
15661576
by_body = {str(r.log_record.body): r for r in ours}
1567-
a_log = by_body["node a executing"]
1568-
b_log = by_body["node b executing"]
1577+
a_log = by_body[node_emit_messages["a"]]
1578+
b_log = by_body[node_emit_messages["b"]]
15691579

15701580
# Invariant: all_logs_same_trace_id.
15711581
trace_ids = {a_log.log_record.trace_id, b_log.log_record.trace_id}

0 commit comments

Comments
 (0)