Skip to content

test: add plugin conformance suite handlers (10-1..10-7) - #580

Merged
wangyb-A merged 4 commits into
mainfrom
conformance-plugin-suite
Jul 28, 2026
Merged

test: add plugin conformance suite handlers (10-1..10-7)#580
wangyb-A merged 4 commits into
mainfrom
conformance-plugin-suite

Conversation

@wangyb-A

@wangyb-A wangyb-A commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

What

7 handlers for the plugin conformance suite (instrumentation-plugin lifecycle hooks) in packages/aws-durable-execution-sdk-python-conformance-tests/handlers/plugin/, plus template_plugin.yaml.

Validation

@wangyb-A
wangyb-A force-pushed the conformance-plugin-suite branch 2 times, most recently from c0a11f7 to 19c7ec1 Compare July 27, 2026 22:59
@wangyb-A wangyb-A changed the title Add plugin conformance suite handlers (10-1..10-7) test: add plugin conformance suite handlers (10-1..10-7) Jul 27, 2026
@wangyb-A
wangyb-A force-pushed the conformance-plugin-suite branch from 19c7ec1 to e5ff099 Compare July 27, 2026 23:50
@wangyb-A
wangyb-A force-pushed the conformance-plugin-suite branch from e5ff099 to 5631d04 Compare July 27, 2026 23:51
@wangyb-A
wangyb-A force-pushed the conformance-plugin-suite branch from 5631d04 to fee0513 Compare July 27, 2026 23:54
@wangyb-A
wangyb-A marked this pull request as ready for review July 27, 2026 23:56
@wangyb-A
wangyb-A temporarily deployed to ai-pr-review-runtime July 27, 2026 23:56 — with GitHub Actions Inactive
@wangyb-A
wangyb-A temporarily deployed to ai-pr-review-runtime July 27, 2026 23:56 — with GitHub Actions Inactive
@github-actions

This comment has been minimized.

@github-actions

Copy link
Copy Markdown

Claude AI review

Review — PR #580: plugin conformance suite handlers (10-1..10-7)

No actionable findings. This PR adds test-only conformance handlers plus template_plugin.yaml; it touches no shippable SDK code and cannot regress runtime behavior. I verified the changed handlers against the checked-out SDK sources and they use the public APIs correctly:

  • durable_execution(plugins=[...]), DurableInstrumentationPlugin, and the hook-info dataclasses (InvocationStartInfo, InvocationEndInfo, OperationStartInfo/EndInfo, UserFunctionStartInfo/EndInfo) all exist with the fields the handlers read (execution_arn, is_first_invocation, status, operation_id, operation_type, attempt, outcome). InvocationEndInfo.status / UserFunctionEndInfo.outcome are non-nullable in the dataclass, so the is not None guards are defensive but harmless.
  • operation_type.name == "STEP" matches OperationType.STEP — correct STEP filtering.
  • StepContext.attempt exists and is 1-based; the 10-3 retry handler mirrors the established step/step_with_retry.py (1-11) pattern (RetryStrategyConfig + create_retry_strategy, retryable_error_types=[RuntimeError]).
  • RetryPresets.none() (10-7) resolves to max_attempts=1, giving the intended terminal FAILED outcome; StepConfig(retry_strategy=...) signature matches.
  • ARN handling is sound: user-function/operation hooks don't carry the ARN, so handlers capture it in on_invocation_start and reuse it, omitting the field when unset rather than inventing one. Replay ordering (10-3: attempt 1 FAILED on first invocation, attempt 2 SUCCEEDED on replay; 10-6: first true→false across the context.wait suspend/replay) is consistent with SDK hook-dispatch semantics.
  • Error isolation (10-4) genuinely exercises PluginExecutor._dispatch_plugin's try/except (plugin.py:305-307), which swallows plugin exceptions — matching the guarantee under test.

Residual test risk (considered, not a defect): template_plugin.yaml omits the LoggingConfig: LogFormat: JSON block that template_step.yaml/template_child.yaml carry. This is consistent with the other 6 templates (map/parallel/callback/invoke/wait/wait_for_condition/wait_for_callback also omit it), and the plugin records are emitted as their own top-level JSON via print(json.dumps(...)) with a top-level durableExecutionArn, so a $.durableExecutionArn filter matches without Lambda's JSON log wrapping. The PR reports 7/7 live passes in us-west-2, corroborating this. No change needed, but if the runner ever standardizes on JSON LogFormat, the top-level durableExecutionArn would become nested under message and the scoping filter would need revisiting.

Reviewed commit fee05131e600b8c1a33e2e6af66c82aa3ddcedac. Workflow run

Alex Wang added 4 commits July 28, 2026 11:11
7 handlers exercising DurableInstrumentationPlugin lifecycle hooks via
durable_execution(plugins=[...]), plus template_plugin.yaml. Validated
live: 7/7 PASS.
One JSON object per hook ({plugin, hook, first|status|n|outcome|op}),
values from the real hook info parameters; replaces packed text lines so
specs assert typed fields directly.
Records carry the execution ARN received via the invocation-start hook
(captured in plugin state for operation/user-function hooks) so the
runner's execution-scoped CloudWatch JSON filter retrieves them.
@wangyb-A
wangyb-A force-pushed the conformance-plugin-suite branch from fee0513 to 2cc14d7 Compare July 28, 2026 18:11
@wangyb-A
wangyb-A had a problem deploying to ai-pr-review-runtime July 28, 2026 18:11 — with GitHub Actions Failure
@wangyb-A
wangyb-A temporarily deployed to ai-pr-review-runtime July 28, 2026 18:11 — with GitHub Actions Inactive
@github-actions

Copy link
Copy Markdown

Codex AI review

No actionable findings found. Residual risk remains around live Lambda/CloudWatch log formatting and the external structured-log matcher, which are not locally covered.

Reviewed commit 2cc14d791f72138468e82501b7bf5ddcfb0c3ba8. Workflow run

@wangyb-A

Copy link
Copy Markdown
Contributor Author

re AI review: We use print + json intentionally, since the test runner is filtering the top level durable execution arn, and the plugin does not have access to the context.logger, the test case print the raw json with durable execution arn so that the test runner can discover and validate the log.

@wangyb-A
wangyb-A merged commit c41894f into main Jul 28, 2026
24 of 26 checks passed
@wangyb-A
wangyb-A deleted the conformance-plugin-suite branch July 28, 2026 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants