Repository navigation
test: cover OTel replay and status mappings - #758
zhongkechen wants to merge 41 commits into
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Represent an exactly empty serialized callback error as absent, matching CallbackDetails.from_dict without requiring a file-store round trip. Preserve every present field, including empty strings and stack lists. Compare real public handler failures across memory/file stores and both OTel views, including caller error/status and replay behavior. Pin the coordinated shared S1c revision with the strict empty-payload, parent-occurrence and callback phase-gate assertions.
This comment has been minimized.
This comment has been minimized.
Match the observed service projection for a failed callback with no error
details when IncludeExecutionData is true: keep Payload {} and mark it
untruncated. SDK-facing state remains error=None, with an UNSET leaf and
the same caller failure. Other statuses, nonempty errors and metadata-only
history retain their existing representation.
Verify the raw public history shape, state/telemetry separation and direct
factory boundaries. The unchanged JS 19ee19f example suite passes all 148
tests without retries or assertion/selection changes.
This comment has been minimized.
This comment has been minimized.
|
@hln33 Follow-up to your #752 coverage comment: this PR now adds case 25 for the missing non-success/no-error-details mapping, alongside the existing case 24 RETRYING/UNSET coverage. Case 25 uses the public callback-failure handler and sends failure without Error after InvocationCompleted. The shared requirement checks the typed empty raw error payload and requires the FAILED callback leaf to remain UNSET in both views. Returning OK for that branch would now fail conformance. Case 26 also exercises a root callback’s first completion, its second-invocation parent, and two subsequent replays without duplicate terminal exports. On current commit cd81a3d, S1c cloud run 37716434728 reports 26/26 passed in all four backend/view suites, with zero failed or uncovered cases; both long-running suites pass 4/4. The unchanged JS examples CI also passes 148 tests on this head. FAILED, CANCELLED, TIMED_OUT and STOPPED without error details remain covered by the two-view unit matrix. The new real-service case exercises FAILED; no cloud reachability claim is made for the other three statuses. The separate new typed-history round-trip review is being investigated as testing-API maintenance; it does not change this deployed handler coverage. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Codex AI reviewNo actionable findings found. Residual risk remains in timing-sensitive worker/context propagation and cloud OTel export ordering, which were reviewed statically. Reviewed commit |
Add Python handlers and SAM mappings for OTel cases 21–26: completed-step replay, active context in user functions and callbacks, invocation retry status, callback failure without error details, and external callback completion followed by two later replays. Handlers use public durable APIs and ordinary implicit span parenting.
Case 26 creates a root callback, saves its result in a durable observation step, and uses two callback barriers to force four invocations. The shared driver delivers callbacks after InvocationCompleted; raw S3 assertions require the first terminal export before the observation step, under the second Invocation in Invocation view, and reject duplicate exports on later replays. Case 25 reuses the callback-failure handler with an omitted payload and strict typed-empty-history-payload precondition.
The testing library reports callback success/failure/timeout through UpdatedOperationIds and consumes only versions delivered by successful checkpoints. Core terminal notifications track actual delivery per invocation, preserving first notifications, concurrent delivery and resets. Exactly empty callback errors become absent in SDK-facing state while detailed history retains Error.Payload={} and Truncated=false, including typed round trips. Present error fields, metadata-only history and checkpoint tokens/watermarks retain their behavior.
A sibling checkpoint can deliver a terminal operation before replay creates its parent span. Invocation view now retains the real completion until that actual parent span is registered, then exports the terminal segment before returning control to the branch. It does not invent ancestors or alter SDK notification accounting. Unresolved parents are reported after normal cleanup/flush, and pending state is cleared on reuse. A deterministic public two-callback regression verifies both original nonterminal and terminal segments, their real parents, the observation boundary and two later replays.
The #756 prerequisite moves Start, handler/finally, existing output/error preparation, resource shutdown and End onto one worker; the caller owns executor shutdown. It removes the extra handler_context API while retaining failed-Start Context/token ownership. Registered-plugin map/parallel submissions receive fresh coordinator Context copies, including resumes, preserving sibling isolation and the no-plugin path. The required reused-worker executor unit test is included. OTel 1.1 requires redesigned core 2.1, released first.
The reusable workflow remains bdb4f1cd0f9252c1aaa978bb8b341b71f2b9d9dc. Runner/catalog default and fallback are 75987d46a915bc37409eed3ea9c3617a924c9756 from aws/aws-durable-execution-conformance-tests#131, including shared #120's 120-second log polling, accumulation and deduplication. The 26 requirements, handlers/templates, backend matrix, long-running scenarios, resource locks and failed-plus-uncovered gates remain intact. The shared self-test fixture stays on its already-validated reference 12e760c; SDK CI tests this PR's exact head.
Merge prerequisites: #756 and aws/aws-durable-execution-conformance-tests#131. Status-mapping prerequisite #752 is merged.
Coverage boundary: case 24 verifies RETRY to RETRYING/UNSET. Case 25 verifies FAILED without error details to UNSET and typed empty history payloads in both views. CANCELLED, TIMED_OUT and STOPPED without error details retain unit coverage; their cloud paths are not established. On preceding head 21be89e, OTel run 37860686614 verifies all four 26/26 reports and both 4/4 long-running reports with no failures or uncovered cases. Both S3 views' actual case-25/26 histories were inspected. Current head d59af28 is now fully green. OTel run 37871824929 resolved runner 75987d46a915bc37409eed3ea9c3617a924c9756 and passes all four 26/26 plus both 4/4 reports (112 cases), with zero failed/uncovered cases. Both S3 views' case-25/26 raw histories were verified. The matching strict reference test is updated and its 83-script-test suite passes.
Validation: