[fix]: various concurrencyOperation bug fixes#247
Conversation
| if (toleratedFailureCount != null && failed > toleratedFailureCount) { | ||
| return ConcurrencyCompletionStatus.FAILURE_TOLERANCE_EXCEEDED; | ||
| } |
There was a problem hiding this comment.
public static CompletionConfig allSuccessful() {
}
return new CompletionConfig(null, null, percentage);
}
We have the above configuration for allSuccessful, does that mean if minSuccessful == null, we need to complete the concurrency opertion when there's any failure?
There was a problem hiding this comment.
allSuccessful is defined as
public static CompletionConfig allSuccessful() {
return new CompletionConfig(null, 0, null);
}
There is no percentage parameter for allSuccessful, so where does it come from?
sdk/src/main/java/software/amazon/lambda/durable/operation/ConcurrencyOperation.java
Show resolved
Hide resolved
sdk/src/main/java/software/amazon/lambda/durable/operation/ConcurrencyOperation.java
Show resolved
Hide resolved
|
This is a huge PR. I will merge the change now to avoid conflicts and unblock the others. Comments are still welcome, and I will follow up to resolve any potential issues. |
| @@ -31,15 +32,26 @@ public String handleRequest(GreetingRequest input, DurableContext context) { | |||
| context.wait(null, Duration.ofSeconds(10)); | |||
There was a problem hiding this comment.
Do we need to remove this? If we are waiting for 10 seconds within step 2.
There was a problem hiding this comment.
step2 is using wait as a at-most timer.
|
|
||
| import java.util.concurrent.atomic.AtomicInteger; | ||
| import org.junit.jupiter.api.Test; | ||
| import software.amazon.lambda.durable.config.StepConfig; |
There was a problem hiding this comment.
Are these placeholder for future changes?
There was a problem hiding this comment.
import wasn't needed because they were in the same package.
| var root = DurableContextImpl.createRootContext( | ||
| executionManager, DurableConfig.builder().build(), null); | ||
| executionManager, | ||
| software.amazon.lambda.durable.DurableConfig.builder().build(), |
There was a problem hiding this comment.
why do we need full path?
There was a problem hiding this comment.
I don't think so. Added probably by IDE
| @@ -1,6 +1,6 @@ | |||
| // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | |||
There was a problem hiding this comment.
Can't we use common config for parallel and map both?
There was a problem hiding this comment.
They are different
3 participants
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Issue Link, if available
paralleloperation #88map#39Description
runUserHandlerConcurrencyOperationDemo/Screenshots
Checklist
Testing
Unit Tests
Have unit tests been written for these changes?
Integration Tests
Have integration tests been written for these changes?
Examples
Has a new example been added for the change? (if applicable)