Docs/migration 1x to 2x - #655
Conversation
31f5f5e to
162361b
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Add docs/migration-1.x-to-2.x.md covering the 2.x breaking changes: typed per-operation error hierarchy, first-run serialize/deserialize round trip, removed config types and fields, construction-time config validation, and wait_for_condition changes; plus the optional should_complete feature. Also document the initial_state serdes requirement on WaitForConditionConfig.
162361b to
e4ae074
Compare
| `2.x` adds a `should_complete` predicate to `CompletionConfig`, giving `map` and | ||
| `parallel` full control over when a batch completes early. This is a new feature, | ||
| not a breaking change - no action is required unless you adopt it. |
There was a problem hiding this comment.
Codex AI review
[P1] This API is not implemented in the 2.x source: CompletionConfig has no should_complete, and the documented helpers and types are neither defined nor exported. Following this section results in ImportError or TypeError. Remove the section, or include the implementation, exports, and tests before documenting it.
| and preserves the original error as `__cause__` (on replay, `__cause__` is | ||
| reconstructed from the checkpointed wire fields `error_type`/`message`/`data`/`stack_trace`). |
There was a problem hiding this comment.
Codex AI review
[P2] The original exception is not preserved as __cause__. raise_as_operation_error() reconstructs a metadata-bearing DurableOperationError on both the first run and replay; for example, a ValueError does not remain a ValueError, and custom attributes are lost. Document the reconstructed stand-in and direct users to error_type, message, data, and stack_trace instead.
Codex AI reviewFound two migration-guide accuracy issues. No runtime behavior changed; Markdown examples remain untested. Reviewed commit |
| ## New in 2.x: Custom Completion Predicate (Optional) | ||
|
|
||
| `2.x` adds a `should_complete` predicate to `CompletionConfig`, giving `map` and | ||
| `parallel` full control over when a batch completes early. This is a new feature, | ||
| not a breaking change - no action is required unless you adopt it. | ||
|
|
||
| ```python | ||
| from aws_durable_execution_sdk_python import complete_batch, continue_batch | ||
|
|
||
| config = CompletionConfig( | ||
| should_complete=lambda status: ( | ||
| complete_batch() if status.success_count >= 2 else continue_batch() | ||
| ) | ||
| ) | ||
| ``` | ||
|
|
||
| The predicate receives a `CompletionStatus` snapshot (counts plus per-item | ||
| statuses) and returns a `CompletionDecision` - `continue_batch()` or | ||
| `complete_batch(outcome)`. The outcome reports `CUSTOM_COMPLETION_SUCCEEDED` or | ||
| `CUSTOM_COMPLETION_FAILED`; a failed custom completion surfaces through | ||
| `throw_if_error()` as a `ChildContextError`, so there is still no separate | ||
| batch-completion error type to catch. Notes: | ||
|
|
||
| - It cannot be combined with `min_successful` or the `tolerated_failure_*` | ||
| fields; doing so raises `ValidationError` at construction. | ||
| - The predicate must be deterministic and side-effect-free. Replay uses the | ||
| checkpointed decision and never re-invokes it. | ||
| - New exports: `complete_batch`, `continue_batch`, `CompletionStatus`, | ||
| `CompletionDecision`, `CompletionOutcome`, `CompletionItemStatus`, | ||
| `BatchItemStatus`. |
There was a problem hiding this comment.
Claude AI review
This entire "Custom Completion Predicate" section documents an API that does not exist in the base source, so a reader following it hits errors immediately:
CompletionConfig(config.py:102) has onlymin_successful,tolerated_failure_count, andtolerated_failure_percentage— there is noshould_completefield, soCompletionConfig(should_complete=...)raisesTypeError: __init__() got an unexpected keyword argument 'should_complete'.complete_batch,continue_batch,CompletionStatus,CompletionDecision,CompletionOutcome, andCompletionItemStatusare defined nowhere inpackages/, sofrom aws_durable_execution_sdk_python import complete_batch, continue_batchraisesImportError.BatchItemStatusdoes exist (concurrency/models.py:33) but is not re-exported from the package root (__init__.py__all__), so it is not importable as shown either.- The claimed
min_successful/tolerated_failure_*mutual-exclusionValidationErroris likewise not implemented inCompletionConfig.__post_init__.
Since the rest of the guide accurately describes the 2.x surface present in this repo, this section stands out as documenting an unshipped feature. Fix: remove this section (and the complete_batch/continue_batch/CompletionStatus/CompletionDecision/CompletionOutcome/CompletionItemStatus/BatchItemStatus "New exports" bullet) until the predicate feature actually lands and is exported, or gate it behind the release that introduces it.
Claude AI reviewThis PR adds a I verified each guide claim against the checked-out base source. The vast majority are accurate: the removed names, the new typed error hierarchy and its package-root exports ( One confirmed defect: the "New in 2.x: Custom Completion Predicate" section documents an API that does not exist in this repository. See Residual test risk: none applicable — this is documentation only. Its correctness cannot be enforced by tests, so the invented section would ship uncaught. Reviewed commit |
Issue #, if available: N/A
Description of changes:
Migration guide for python v2
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.