Repository navigation
feat(sdk): custom subtypes typed str - #837
Conversation
8827d84 to
1402520
Compare
This comment has been minimized.
This comment has been minimized.
1402520 to
483c445
Compare
This comment has been minimized.
This comment has been minimized.
ParidelPooya
left a comment
There was a problem hiding this comment.
Approving. I found no blocking defect. With no subtype configured, the recorded subtypes are identical to main, and the state-load failure from #749 is fixed. Four non-blocking items follow. I would act on item 1.
1. An invalid subtype on a default step runs the step body, then fails the execution
- The SDK does not validate
sub_type. It relies on the service to reject a badSubTypewith a 400. - A step with the default
AT_LEAST_ONCE_PER_RETRYsends its START checkpoint without waiting (operation/step.py:158-171). So the step body runs before the service answers. - The SDK classifies a 4xx checkpoint error as an EXECUTION error (
exceptions.py:163). So the execution then fails terminally. - So a typo in a subtype runs the side effect, for example a payment, and then fails the execution.
I patched the local checkpoint validator to reject subtypes the way the service does (1 to 32 characters of [A-Za-z0-9_-]) and ran one step:
AT_LEAST_ONCE_PER_RETRY sub_type='OrderCharge' status=SUCCEEDED body_runs=1
AT_LEAST_ONCE_PER_RETRY sub_type='Order Charge' status=FAILED body_runs=5
AT_MOST_ONCE_PER_RETRY sub_type='Order Charge' status=FAILED body_runs=0
The count of 5 comes from the local runner re-invoking. I did not measure the cloud count. The code path above gives at least 1.
#749 step 4 asked the SDK to reject such a value with a clear error. This PR chose JS parity instead, and I confirmed the JS SDK on main has no format check either. Suggestion: validate sub_type in __post_init__ of StepConfig, CallbackConfig, and ChildConfig. Then the error is raised when the config is built, before any checkpoint, and the wire behavior does not change. If parity matters more, the same item could be opened on the JS SDK, or #749 could state that step 4 is deliberately dropped.
The same check would also reject non-str values. Today OperationIdentifier(sub_type=123) puts the integer 123 into the SubType field of the update dict.
2. A caller can record an SDK-reserved subtype
ChildConfig(sub_type="ParallelBranch") and StepConfig(sub_type="WaitForCondition") are accepted, and the execution succeeds. A consumer that filters by subtype then counts these operations as SDK operations. One such consumer is in this repo: conformance-tests/handlers/plugin/plugin_parallel_branch_hooks.py:37. The Go SDK rejects reserved values. If item 1 adds validation, it could reject the OperationSubType values too. Otherwise, one docstring line could warn against reusing them.
3. An OperationSubType member passed by a caller is not converted to str
ChildConfig.sub_typestill accepts anOperationSubTypemember, which is now aStrEnum.run_in_child_contextkeeps the member as it is. So the identifier, the update, and the first invocation's plugin events carry aStrEnuminstance (<OperationSubType.PARALLEL_BRANCH: 'ParallelBranch'>).- After a reload from the service, the same field is a plain
str. ==works in both cases. But theOperationInfo.sub_typedocstring promises a plainstr, andtype(x) is stris false on the first invocation.
Suggestion: apply str(...) where step, create_callback, and run_in_child_context resolve the subtype.
4. Release order
The testing package now requires core >=3.0.0. PyPI has core up to 2.1.0. So hatch run test:cov from inside packages/aws-durable-execution-sdk-python-testing cannot resolve dependencies today. The root workspace environment works. The release order is: core 3.0.0, then testing, then the emulator image.
What I verified
Each check ran against this branch and against main.
- Default subtypes are unchanged. One handler ran step, wait, callback,
wait_for_callback,wait_for_condition, child context, virtual child context, map, and parallel. All 36 history rows of (name, event type, subtype) are identical on both branches. I checked invoke by reading the code only. - The state-load fix works. On
main,Operation.from_dictwithSubType="OrderSaga"raisesValueError. On this branch it returns'OrderSaga'. - Replay detects a changed subtype. A step recorded
OrderCharge. Its subtype changed toPaymentChargewhile the execution was suspended on a callback. The next invocation failed withNonDeterministicExecutionError(subtype checkpoint='OrderCharge' current='PaymentCharge'). The step body ran once. - The emulator now parses a custom subtype. On
main,CheckpointDurableExecutionRequest.from_dictraisesValueErrorforSubType="OrderCharge", which the JS SDK can already send. On this branch it parses. So this PR also fixes the local emulator for JS SDK callers who use custom subtypes. - Positional order is preserved.
WaitForCallbackConfig(timeout, heartbeat_timeout, serdes, retry_strategy)still bindsretry_strategy.ChildConfig(None, "X").sub_type == "X". - An empty string selects the default for step, callback, and child context.
- No SDK logic branches on a subtype value. The only comparison in any package's
srcis the conformance handler named in item 2. hatch run test:cov: 5986 passed, 4 skipped, 98% coverage.hatch run types:checkis clean.hatch fmt --checkis clean for the PR's files.- I did not run anything in the cloud.
Subtype fields were typed OperationSubType, and the state loader raised ValueError on any other value. So a caller could not label a step, callback, or child context, and an execution that recorded a custom label could not load its own state on the next invocation. A subtype is now a plain str. OperationSubType is a StrEnum of the values the SDK records for its own operations; compare with ==. The state loader keeps the string the service stored. OperationIdentifier holds the plain value, so a loaded checkpoint and a running operation compare and print alike, and replay checks subtypes by value. StepConfig and CallbackConfig gain a keyword-only sub_type. ChildConfig.sub_type is a str. None or "" selects the default. The configs raise ValidationError for a value the service would reject, so a step body never runs under a bad subtype. Every OperationUpdate factory records identifier.sub_type, so the create_context_* factories lose their sub_type parameter and ChildOperationExecutor its sub_type attribute. OTel, the telemetry exporter, the testing runner, and the conformance handlers read the subtype as a string. The testing package requires core 3.0.0. Docs: aws/aws-durable-execution-docs#299. BREAKING CHANGE: Operation.sub_type, OperationUpdate.sub_type, ChildConfig.sub_type, OperationInfo.sub_type, and the testing runner's Operation.sub_type are str | None. Code that reads .value or compares with `is` must compare with == instead. OperationUpdate.create_context_start/succeed/fail lose their sub_type parameter. aws_durable_execution_sdk_python.plugin no longer re-exports OperationSubType; import it from lambda_service. Closes #749
483c445 to
b781a2f
Compare
|
Thanks for the thorough review. Addressed in b781a2f:
|
Codex AI reviewNo actionable findings. Review was limited to static inspection; runtime and integration behavior were not independently tested. Reviewed commit |
The guide named one breaking change. Main now has three: plugin factories (#737), subtypes as strings (#837), and None kept in batch results (#765). Add a section for each of the two new ones, with the searches that find affected code. The dependency table moves testing to 1.4.0 and adds the telemetry exporter at 0.0.2, the versions #841 publishes. The lifecycle section states where each hook runs after plugin hooks moved off the checkpoint thread.
The guide named one breaking change. Main now has three: plugin factories (#737), subtypes as strings (#837), and None kept in batch results (#765). Add a section for each of the two new ones, with the searches that find affected code. The dependency table moves testing to 1.4.0 and adds the telemetry exporter at 0.0.2, the versions #841 publishes. The lifecycle section states where each hook runs after plugin hooks moved off the checkpoint thread.
The guide named one breaking change. Main now has three: plugin factories (#737), subtypes as strings (#837), and None kept in batch results (#765). Add a section for each of the two new ones, with the searches that find affected code. The dependency table moves testing to 1.4.0 and adds the telemetry exporter at 0.0.2, the versions #841 publishes. The lifecycle section states where each hook runs after plugin hooks moved off the checkpoint thread.
The guide named one breaking change. Main now has three: plugin factories (#737), subtypes as strings (#837), and None kept in batch results (#765). Add a section for each of the two new ones, with the searches that find affected code. The dependency table moves testing to 1.4.0 and adds the telemetry exporter at 0.0.2, the versions #841 publishes. The lifecycle section states where each hook runs after plugin hooks moved off the checkpoint thread.
Closes #749. Part of the 3.0 major release; ships with #801.
What changes
A subtype is a plain
str. Every subtype field is typedstr | None:Operation.sub_type,OperationUpdate.sub_type,ChildConfig.sub_type, pluginOperationInfo.sub_type, and the testing runner'sOperation.sub_type.OperationIdentifier.sub_typeisstr.OperationSubTypestays, as aStrEnumof the values the SDK records for its own operations. Compare with==:operation.sub_type == OperationSubType.MAPis true for the operationcontext.map()records, andoperation.sub_type == "OrderSaga"for a child context configured withChildConfig(sub_type="OrderSaga").StepConfigandCallbackConfiggain a keyword-onlysub_type.ChildConfig.sub_typekeeps its positional slot; only its type changed.WaitForCallbackConfiginheritssub_typeand labels the callback the operation creates; the surrounding context keepsWaitForCallback.Noneor""selects the default.The state loader keeps the subtype string the service stored. Before,
Operation.from_dictraisedValueErroron any value outside the enum, so an execution that recorded a custom subtype could not load its own state on the next invocation.Every
OperationUpdate.create_*factory recordsidentifier.sub_type, socreate_context_start/succeed/faillose their separatesub_typeparameter andChildOperationExecutoritssub_typeattribute. The subtype lives in one place.OTel, the telemetry exporter, the testing runner, and the conformance handlers read the subtype as a string. The testing package requires core
>=3.0.0.Decisions
SubTypeoutside its model constraint withValidationExceptionand the execution fails. The local checkpoint server does not yet mirror that rejection: [Testing]: Local checkpoint server accepts a SubType the service rejects #836.aws_durable_execution_sdk_python.pluginno longer importsOperationSubType. It was never exported from there; import it fromlambda_service.Migration guide input for #801
str | None. Replace.valuereads with the value itself andiscomparisons with==.OperationUpdate.create_context_start(identifier),create_context_succeed(identifier, payload, *, context_options=None), andcreate_context_fail(identifier, error)lose theirsub_typeparameter.ChildConfig.sub_typeaccepts any string the service accepts.OperationSubTypefromaws_durable_execution_sdk_python.lambda_service.Operation.sub_typetype changed.Testing
hatch run test:cov, every package: 5987 passed, 4 skipped, 98% coverage.hatch run types:checkclean.hatch fmt --checkclean in all 10 packages.test-wheel-otelandtest-wheel-otel-legacypass.tests/e2e/custom_subtype_int_test.py: custom subtypes on a step, a callback, and a child context through the local runner, across a suspend and replay, checked in the runner result, the execution history, and plugin events. Newexecution_testreplays state loaded from service-shaped dicts with the SDK's own subtypes as strings (RunInChildContext,Map,MapIteration,Step) without re-running any body.hatch run test:examples-integration69 passed, 12 skipped (same skips asmain). A probe function with custom subtypes on a step, a callback, and a child context completedSUCCEEDED; the execution history recordedOrderSaga,OrderCharge, andOrderApprovalon the expected events.No conformance tests were changed beyond the five plugin handlers reading
sub_typeas a string.