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.
162361b to
e4ae074
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
e4ae074 to
2681198
Compare
This comment has been minimized.
This comment has been minimized.
fbc460e to
698249c
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.
698249c to
1e93efe
Compare
This comment has been minimized.
This comment has been minimized.
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
Claude AI reviewThis PR adds a 1.x→2.x migration guide ( I verified every concrete technical claim in the guide against the base-branch (2.x) source, since a migration guide's correctness is its whole value:
Example imports are correct (e.g. No actionable correctness, determinism, API-compatibility, or typing issues were found in the diff. Residual notes (non-blocking): docs-only change, so no tests are required under the repo policy; the new guide is not linked from any index in the diff, and the PR is still marked draft — neither is a defect in the changed lines. Reviewed commit |
| | `WaitDecision` removed; `WaitStrategyConfig.timeout` / `timeout_seconds` removed | Use `WaitForConditionDecision` (`stop_polling()` / `continue_waiting(delay)`). | | ||
| | `wait_for_condition` raises `WaitForConditionError` when it exhausts `max_attempts` | Catch `WaitForConditionError` instead of inspecting the returned state. | | ||
|
|
||
| Find affected code before upgrading: |
There was a problem hiding this comment.
Codex AI review
[P1] Document that the default map configuration is affected too. context.map(..., config=None) constructs MapConfig(), whose all-None CompletionConfig is treated as fail-fast in 2.x. Callers relying on the 1.x default can therefore stop after the first failed item even though they never hand-built a config. Explicitly direct them to MapConfig(completion_config=CompletionConfig.all_completed()) to preserve process-all behavior.
| item/branch failures still surface as `ChildContextError`. Notes: | ||
|
|
||
| - It cannot be combined with `min_successful` or the `tolerated_failure_*` | ||
| fields; doing so raises `ValidationError` at construction. | ||
| - The predicate must be deterministic, side-effect-free, and monotonic: once a | ||
| progress snapshot returns `complete_batch(outcome)`, every later snapshot | ||
| containing that progress must return `complete_batch(outcome)` with the same |
There was a problem hiding this comment.
Codex AI review
[P2] Document the predicate's initial invocation. should_complete runs before any branch is scheduled, with completed_count == 0 and unscheduled item statuses set to None, and also runs on suspension state changes. Without this, a predicate may dereference a missing status, fail the batch, or return complete_batch() immediately and execute no branches. Add these invocation and None-handling requirements.
Codex AI reviewTwo migration-guide gaps could cause upgraded workflows to skip work or fail unexpectedly. The Python change is documentation-only; no runtime tests were executed under the review constraints. 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.