generated from amazon-archives/__template_Apache-2.0
-
Notifications
You must be signed in to change notification settings - Fork 4
fix(sdk): fix minSuccessful issue in map/parallel #378
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
ParidelPooya
wants to merge
10
commits into
main
Choose a base branch
from
fix/min-successful-filtering
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+257
−575
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…hedAncestor functionality - Removed pendingCompletions Set from ExecutionContext - Removed hasPendingAncestorCompletion and hasFinishedAncestor methods from CheckpointManager - Removed ancestor checking logic from checkpoint operations - Removed related unit tests that were testing the removed functionality - All main SDK tests now pass (725/725) - 8 callback-related example tests are failing due to operation lifecycle issues
…on cleanup - Remove operation deletion from CheckpointManager to prevent race conditions - Keep operations in map while cleaning up resources to maintain lifecycle - Resolves issue where callback operations failed when parent contexts completed - All 8 previously failing wait-for-callback tests now pass
…/map-min-successful
- Add markOperationState calls to track operation lifecycle - Mark operations as IDLE_NOT_AWAITED when started - Mark operations as COMPLETED when finished (success or failure) - Update test mock to include markOperationState method - Maintains consistency with other handlers like wait-handler - All tests passing (69/69 test suites, 725/725 tests)
- Add CHILD_IS_EXECUTING state to OperationLifecycleState enum - Add metadata to all markOperationState calls across all handlers: - callback-promise.ts: 2 calls with proper operation metadata - invoke-handler.ts: 2 calls with CHAINED_INVOKE metadata - run-in-child-context-handler.ts: 3 calls with CONTEXT metadata - wait-for-condition-handler.ts: 2 calls with WAIT_FOR_CONDITION metadata - step-handler.ts: 3 calls with STEP metadata - wait-handler.ts: 1 call with WAIT metadata - Add ancestor completion check in checkpoint() method - Update cleanup logic to include CHILD_IS_EXECUTING state - Fix 'metadata required on first call' errors in callback tests - Ensure consistent operation lifecycle management across all handlers
…om being sent to checkpoint - Add ancestor completion check in processBatch to filter out operations - Operations with completed ancestors are resolved locally instead of sent to testing library - Fixes issue where remaining operations showed SUCCEEDED instead of STARTED in minSuccessful scenarios - Resolves parallel and map minSuccessful test failures
- Filter only START actions when ancestor is completed to prevent operations from updating after parent completion - Allow completion events (SUCCEED/FAIL) to pass through to maintain correct execution history - Remove debug logging statements that were interfering with tests - MinSuccessful tests now pass correctly with remaining operations staying in STARTED state
- Add batch-level filtering in processBatch to prevent operations from being sent to testing library when their ancestor has already completed - This fixes the issue where remaining operations in minSuccessful scenarios were showing SUCCEEDED status instead of STARTED - The filtering ensures that once a parent context completes (e.g., due to minSuccessful threshold), child operations don't send updates - MinSuccessful tests now pass correctly with proper operation status tracking
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fix minSuccessful issue in map/parallel