Skip to content

Docs/migration 1x to 2x - #655

Open
ayushiahjolia wants to merge 1 commit into
mainfrom
docs/migration-1x-to-2x
Open

Docs/migration 1x to 2x#655
ayushiahjolia wants to merge 1 commit into
mainfrom
docs/migration-1x-to-2x

Conversation

@ayushiahjolia

Copy link
Copy Markdown
Contributor

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.

@ayushiahjolia
ayushiahjolia force-pushed the docs/migration-1x-to-2x branch from 31f5f5e to 162361b Compare August 18, 2026 21:38
@ayushiahjolia
ayushiahjolia marked this pull request as ready for review August 18, 2026 21:39
@ayushiahjolia
ayushiahjolia temporarily deployed to ai-pr-review-runtime August 18, 2026 21:39 — with GitHub Actions Inactive
@ayushiahjolia
ayushiahjolia temporarily deployed to ai-pr-review-runtime August 18, 2026 21:39 — with GitHub Actions Inactive
Comment thread docs/migration-1.x-to-2.x.md
Comment thread docs/migration-1.x-to-2.x.md Outdated
Comment thread docs/migration-1.x-to-2.x.md Outdated
@github-actions

This comment has been minimized.

Comment thread docs/migration-1.x-to-2.x.md
Comment thread docs/migration-1.x-to-2.x.md Outdated
@github-actions

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.
@ayushiahjolia
ayushiahjolia force-pushed the docs/migration-1x-to-2x branch from 162361b to e4ae074 Compare August 19, 2026 20:57
@ayushiahjolia
ayushiahjolia temporarily deployed to ai-pr-review-runtime August 19, 2026 20:57 — with GitHub Actions Inactive
@ayushiahjolia
ayushiahjolia temporarily deployed to ai-pr-review-runtime August 19, 2026 20:57 — with GitHub Actions Inactive
Comment on lines +114 to +116
`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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +41 to +42
and preserves the original error as `__cause__` (on replay, `__cause__` is
reconstructed from the checkpointed wire fields `error_type`/`message`/`data`/`stack_trace`).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@github-actions

Copy link
Copy Markdown
Contributor

Codex AI review

Found two migration-guide accuracy issues. No runtime behavior changed; Markdown examples remain untested.

Reviewed commit e4ae074b14d2782d0951c695ad5656949a13d765. Workflow run

Comment on lines +112 to +141
## 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`.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 only min_successful, tolerated_failure_count, and tolerated_failure_percentage — there is no should_complete field, so CompletionConfig(should_complete=...) raises TypeError: __init__() got an unexpected keyword argument 'should_complete'.
  • complete_batch, continue_batch, CompletionStatus, CompletionDecision, CompletionOutcome, and CompletionItemStatus are defined nowhere in packages/, so from aws_durable_execution_sdk_python import complete_batch, continue_batch raises ImportError.
  • BatchItemStatus does 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-exclusion ValidationError is likewise not implemented in CompletionConfig.__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.

@github-actions

Copy link
Copy Markdown
Contributor

Claude AI review

This PR adds a docs/migration-1.x-to-2.x.md guide and a docstring for WaitForConditionConfig in waits.py.

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 (exceptions.py, __init__.py), TerminationReason no longer having a CALLBACK_ERROR member, BatchResult.throw_if_error() raising ChildContextError (concurrency/models.py:382), the first-run serialize/deserialize round trip (operation/child.py, operation/wait_for_condition.py), config __post_init__ validation, CompletionConfig.all_completed() semantics (config.py:193), and wait_for_condition raising WaitForConditionError on attempt exhaustion (waits.py:76). The new WaitForConditionConfig docstring correctly matches the implementation.

One confirmed defect: the "New in 2.x: Custom Completion Predicate" section documents an API that does not exist in this repository. See docs/migration-1.x-to-2.x.md:112. Following it produces an immediate ImportError/TypeError.

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 e4ae074b14d2782d0951c695ad5656949a13d765. Workflow run

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.

1 participant