Skip to content

Commit f3cdbbc

Browse files
siri-varmamcruzdevartur-ciocanuArtur Ciocanucicoyle
committed
Fix the issue with retries not happening correctly for Activities and Workflows (dapr#1343)
* Add coverage for some properties (dapr#1297) Signed-off-by: sirivarma <[email protected]> * Make the DAPR version being used consistent across all tests (dapr#1299) Signed-off-by: sirivarma <[email protected]> * Separate Dapr constants from IT container constants (dapr#1315) Signed-off-by: Artur Ciocanu <[email protected]> Co-authored-by: Artur Ciocanu <[email protected]> Signed-off-by: sirivarma <[email protected]> * Use Java Bean for connection details and add more tests (dapr#1317) * Use Java Bean for connection details and add more tests Signed-off-by: Artur Ciocanu <[email protected]> * Simplify mock setup Signed-off-by: Artur Ciocanu <[email protected]> * Adding even more tests for test coverage Signed-off-by: Artur Ciocanu <[email protected]> --------- Signed-off-by: Artur Ciocanu <[email protected]> Co-authored-by: Artur Ciocanu <[email protected]> Co-authored-by: Cassie Coyle <[email protected]> Signed-off-by: sirivarma <[email protected]> * Update CONTRIBUTING.md Signed-off-by: Siri Varma Vegiraju <[email protected]> Signed-off-by: sirivarma <[email protected]> * Bump codecov/codecov-action from 5.4.0 to 5.4.2 (dapr#1318) Bumps [codecov/codecov-action](https://github.com/codecov/codecov-action) from 5.4.0 to 5.4.2. - [Release notes](https://github.com/codecov/codecov-action/releases) - [Changelog](https://github.com/codecov/codecov-action/blob/main/CHANGELOG.md) - [Commits](codecov/codecov-action@v5.4.0...v5.4.2) --- updated-dependencies: - dependency-name: codecov/codecov-action dependency-version: 5.4.2 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Cassie Coyle <[email protected]> Co-authored-by: Dapr Bot <[email protected]> Signed-off-by: sirivarma <[email protected]> * Fix URL building logic (dapr#1320) * Fix URL building logic Signed-off-by: Artur Ciocanu <[email protected]> * Add test for query params Signed-off-by: Artur Ciocanu <[email protected]> * Fix the assertion in the test Signed-off-by: Artur Ciocanu <[email protected]> * Adjust the tests Signed-off-by: Artur Ciocanu <[email protected]> * Remove uneeded changes from IT test Signed-off-by: Artur Ciocanu <[email protected]> * Revert some unintended changes Signed-off-by: Artur Ciocanu <[email protected]> * Simplify the testing a little bit Signed-off-by: Artur Ciocanu <[email protected]> * Adjust the test to use ServerRequest Signed-off-by: Artur Ciocanu <[email protected]> * Test removing things from method invoke controller Signed-off-by: Artur Ciocanu <[email protected]> * Add query param encoding test Signed-off-by: Artur Ciocanu <[email protected]> * Revert some unintended changes Signed-off-by: Artur Ciocanu <[email protected]> * Some tiny styles Signed-off-by: Artur Ciocanu <[email protected]> --------- Signed-off-by: Artur Ciocanu <[email protected]> Co-authored-by: Artur Ciocanu <[email protected]> Signed-off-by: sirivarma <[email protected]> * Generate updated javadocs for 1.14.1 Signed-off-by: Dapr Bot <[email protected]> Signed-off-by: sirivarma <[email protected]> * Add Conversation AI to Java SDK (dapr#1235) * Conversation first commit Signed-off-by: Siri Varma Vegiraju <[email protected]> Signed-off-by: sirivarma <[email protected]> Signed-off-by: siri-varma <[email protected]> * Add unit tests Signed-off-by: sirivarma <[email protected]> Signed-off-by: siri-varma <[email protected]> * change ai to conv Signed-off-by: sirivarma <[email protected]> Signed-off-by: siri-varma <[email protected]> * Move to single module Signed-off-by: sirivarma <[email protected]> Signed-off-by: siri-varma <[email protected]> * Remove module Signed-off-by: sirivarma <[email protected]> Signed-off-by: siri-varma <[email protected]> * Add Integration tests Signed-off-by: siri-varma <[email protected]> * Update sdk-tests/src/test/java/io/dapr/it/testcontainers/DaprConversationIT.java Co-authored-by: Cassie Coyle <[email protected]> Signed-off-by: Siri Varma Vegiraju <[email protected]> Signed-off-by: siri-varma <[email protected]> * Fix things Signed-off-by: siri-varma <[email protected]> * Address comments Signed-off-by: siri-varma <[email protected]> * Import tag Signed-off-by: siri-varma <[email protected]> * Address comments Signed-off-by: siri-varma <[email protected]> * Make common config Signed-off-by: siri-varma <[email protected]> * Address comments Signed-off-by: siri-varma <[email protected]> * fix constant Signed-off-by: siri-varma <[email protected]> * fix constant Signed-off-by: siri-varma <[email protected]> * fix constant Signed-off-by: siri-varma <[email protected]> * fix s Signed-off-by: siri-varma <[email protected]> * Fix things Signed-off-by: siri-varma <[email protected]> * Fix things Signed-off-by: siri-varma <[email protected]> * Fix things Signed-off-by: siri-varma <[email protected]> * Make common config Signed-off-by: siri-varma <[email protected]> * Update README.md Signed-off-by: Siri Varma Vegiraju <[email protected]> * Update README.md Signed-off-by: Siri Varma Vegiraju <[email protected]> --------- Signed-off-by: Siri Varma Vegiraju <[email protected]> Signed-off-by: sirivarma <[email protected]> Signed-off-by: siri-varma <[email protected]> Signed-off-by: Siri Varma Vegiraju <[email protected]> Co-authored-by: Cassie Coyle <[email protected]> Co-authored-by: Cassie Coyle <[email protected]> Signed-off-by: sirivarma <[email protected]> * Add docs for usage of Jobs SDK (dapr#1323) * Add doc for jobs Signed-off-by: siri-varma <[email protected]> * Add docs for Jobs Signed-off-by: siri-varma <[email protected]> * Apply suggestions from code review Co-authored-by: Cassie Coyle <[email protected]> Signed-off-by: Siri Varma Vegiraju <[email protected]> --------- Signed-off-by: siri-varma <[email protected]> Signed-off-by: Siri Varma Vegiraju <[email protected]> Co-authored-by: artur-ciocanu <[email protected]> Co-authored-by: Cassie Coyle <[email protected]> Signed-off-by: sirivarma <[email protected]> * Use dapr/durabletask-java (dapr#1336) * microsoft durabletask-java -> dapr durabletask-java Signed-off-by: Cassandra Coyle <[email protected]> * update another ref Signed-off-by: Cassandra Coyle <[email protected]> * 1.5.2 release Signed-off-by: Cassandra Coyle <[email protected]> * fix import order Signed-off-by: Cassandra Coyle <[email protected]> * Sdk new changes Signed-off-by: siri-varma <[email protected]> * Refine workflows Signed-off-by: siri-varma <[email protected]> * add ; Signed-off-by: Cassandra Coyle <[email protected]> * rm try Signed-off-by: Cassandra Coyle <[email protected]> --------- Signed-off-by: Cassandra Coyle <[email protected]> Signed-off-by: siri-varma <[email protected]> Co-authored-by: siri-varma <[email protected]> Signed-off-by: sirivarma <[email protected]> * Update master version to 1.16.0-SNAPSHOT Signed-off-by: Dapr Bot <[email protected]> Signed-off-by: sirivarma <[email protected]> * Fix NPE Signed-off-by: siri-varma <[email protected]> Signed-off-by: sirivarma <[email protected]> * Fix NPE Signed-off-by: siri-varma <[email protected]> Signed-off-by: sirivarma <[email protected]> * Fix NPE Signed-off-by: siri-varma <[email protected]> Signed-off-by: sirivarma <[email protected]> * Fix NPE Signed-off-by: siri-varma <[email protected]> Signed-off-by: sirivarma <[email protected]> * Fix NPE Signed-off-by: siri-varma <[email protected]> Signed-off-by: sirivarma <[email protected]> * Fix NPE Signed-off-by: siri-varma <[email protected]> Signed-off-by: sirivarma <[email protected]> * Fix things Signed-off-by: siri-varma <[email protected]> Signed-off-by: sirivarma <[email protected]> * Renaming and exposing connection details (dapr#1341) Signed-off-by: Artur Ciocanu <[email protected]> Co-authored-by: Artur Ciocanu <[email protected]> Signed-off-by: sirivarma <[email protected]> * [Master] Fix Vulnerabilities (dapr#1354) * update okio Signed-off-by: Cassandra Coyle <[email protected]> * rm unused dep Signed-off-by: Cassandra Coyle <[email protected]> --------- Signed-off-by: Cassandra Coyle <[email protected]> Signed-off-by: sirivarma <[email protected]> * Feat Add TLS & mTLS support for gRPC with root CA and insecure mode (dapr#1361) * feat: Support for GRPC ssl Signed-off-by: Javier Aliaga <[email protected]> * add tests Signed-off-by: Cassandra Coyle <[email protected]> * fix CI Signed-off-by: Cassandra Coyle <[email protected]> * add back else if Signed-off-by: Cassandra Coyle <[email protected]> * channel cleanup Signed-off-by: Cassandra Coyle <[email protected]> * add root ca support Signed-off-by: Cassandra Coyle <[email protected]> * checkstyles Signed-off-by: Cassandra Coyle <[email protected]> * add insecure Signed-off-by: Cassandra Coyle <[email protected]> * fix checkstyles Signed-off-by: Cassandra Coyle <[email protected]> * use InsecureTrustManagerFactory Signed-off-by: Cassandra Coyle <[email protected]> * fix test Signed-off-by: Cassandra Coyle <[email protected]> --------- Signed-off-by: Javier Aliaga <[email protected]> Signed-off-by: Cassandra Coyle <[email protected]> Co-authored-by: Javier Aliaga <[email protected]> Signed-off-by: sirivarma <[email protected]> * Address comments Signed-off-by: siri-varma <[email protected]> Signed-off-by: sirivarma <[email protected]> * Fix things Signed-off-by: siri-varma <[email protected]> Signed-off-by: sirivarma <[email protected]> * Fix things Signed-off-by: siri-varma <[email protected]> Signed-off-by: sirivarma <[email protected]> --------- Signed-off-by: sirivarma <[email protected]> Signed-off-by: Artur Ciocanu <[email protected]> Signed-off-by: Siri Varma Vegiraju <[email protected]> Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: Dapr Bot <[email protected]> Signed-off-by: Siri Varma Vegiraju <[email protected]> Signed-off-by: siri-varma <[email protected]> Signed-off-by: Cassandra Coyle <[email protected]> Signed-off-by: Javier Aliaga <[email protected]> Co-authored-by: Matheus Cruz <[email protected]> Co-authored-by: artur-ciocanu <[email protected]> Co-authored-by: Artur Ciocanu <[email protected]> Co-authored-by: Cassie Coyle <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Dapr Bot <[email protected]> Co-authored-by: Dapr Bot <[email protected]> Co-authored-by: Cassie Coyle <[email protected]> Co-authored-by: Javier Aliaga <[email protected]>
1 parent 8031d28 commit f3cdbbc

File tree

6 files changed

+69
-5
lines changed

6 files changed

+69
-5
lines changed

examples/src/main/java/io/dapr/examples/workflows/childworkflow/DemoChildWorkflow.java

+12-1
Original file line numberDiff line numberDiff line change
@@ -15,18 +15,29 @@
1515

1616
import io.dapr.workflows.Workflow;
1717
import io.dapr.workflows.WorkflowStub;
18+
import io.dapr.workflows.WorkflowTaskOptions;
19+
import io.dapr.workflows.WorkflowTaskRetryPolicy;
20+
21+
import java.time.Duration;
1822

1923
public class DemoChildWorkflow implements Workflow {
2024
@Override
2125
public WorkflowStub create() {
2226
return ctx -> {
2327
ctx.getLogger().info("Starting ChildWorkflow: " + ctx.getName());
2428

29+
WorkflowTaskRetryPolicy policy = WorkflowTaskRetryPolicy.newBuilder()
30+
.setFirstRetryInterval(Duration.ofSeconds(1))
31+
.setMaxNumberOfAttempts(10)
32+
.build();
33+
34+
WorkflowTaskOptions options = new WorkflowTaskOptions(policy);
35+
2536
var childWorkflowInput = ctx.getInput(String.class);
2637
ctx.getLogger().info("ChildWorkflow received input: " + childWorkflowInput);
2738

2839
ctx.getLogger().info("ChildWorkflow is calling Activity: " + ReverseActivity.class.getName());
29-
String result = ctx.callActivity(ReverseActivity.class.getName(), childWorkflowInput, String.class).await();
40+
String result = ctx.callActivity(ReverseActivity.class.getName(), childWorkflowInput, options, String.class).await();
3041

3142
ctx.getLogger().info("ChildWorkflow finished with: " + result);
3243
ctx.complete(result);

examples/src/main/java/io/dapr/examples/workflows/childworkflow/DemoChildWorkflowWorker.java

+1
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ public static void main(String[] args) throws Exception {
3232

3333
// Build and then start the workflow runtime pulling and executing tasks
3434
WorkflowRuntime runtime = builder.build();
35+
runtime.start();
3536
System.out.println("Start workflow runtime");
3637
}
3738
}

sdk-workflows/pom.xml

+1-1
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@
4747
<dependency>
4848
<groupId>io.dapr</groupId>
4949
<artifactId>durabletask-client</artifactId>
50-
<version>1.5.2</version>
50+
<version>1.5.3</version>
5151
</dependency>
5252
<!--
5353
manually declare durabletask-client's jackson dependencies

sdk-workflows/src/main/java/io/dapr/workflows/WorkflowTaskRetryPolicy.java

+3-2
Original file line numberDiff line numberDiff line change
@@ -166,9 +166,10 @@ public Builder setMaxRetryInterval(@Nullable Duration maxRetryInterval) {
166166
* @return This builder
167167
*/
168168
public Builder setRetryTimeout(Duration retryTimeout) {
169-
if (retryTimeout != null && retryTimeout.compareTo(this.firstRetryInterval) < 0) {
169+
if (retryTimeout == null || retryTimeout.compareTo(this.firstRetryInterval) < 0) {
170170
throw new IllegalArgumentException(
171-
"The value for retryTimeout must be greater than or equal to the value for firstRetryInterval.");
171+
"The value for retryTimeout cannot be null and"
172+
+ " must be greater than or equal to the value for firstRetryInterval.");
172173
}
173174

174175
this.retryTimeout = retryTimeout;

sdk-workflows/src/main/java/io/dapr/workflows/runtime/DefaultWorkflowContext.java

+3-1
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,9 @@ private static TaskOptions toTaskOptions(WorkflowTaskOptions options) {
240240
);
241241

242242
retryPolicy.setBackoffCoefficient(workflowTaskRetryPolicy.getBackoffCoefficient());
243-
retryPolicy.setRetryTimeout(workflowTaskRetryPolicy.getRetryTimeout());
243+
if (workflowTaskRetryPolicy.getRetryTimeout() != null) {
244+
retryPolicy.setRetryTimeout(workflowTaskRetryPolicy.getRetryTimeout());
245+
}
244246

245247
return new TaskOptions(retryPolicy);
246248
}

sdk-workflows/src/test/java/io/dapr/workflows/DefaultWorkflowContextTest.java

+49
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,7 @@ public void callChildWorkflowWithOptions() {
304304

305305
assertEquals(retryPolicy.getMaxNumberOfAttempts(), taskOptions.getRetryPolicy().getMaxNumberOfAttempts());
306306
assertEquals(retryPolicy.getFirstRetryInterval(), taskOptions.getRetryPolicy().getFirstRetryInterval());
307+
assertEquals(Duration.ZERO, taskOptions.getRetryPolicy().getRetryTimeout());
307308
}
308309

309310
@Test
@@ -327,4 +328,52 @@ public void newUuidTestNoImplementationExceptionTest() {
327328
String expectedMessage = "No implementation found.";
328329
assertEquals(expectedMessage, runtimeException.getMessage());
329330
}
331+
332+
@Test
333+
public void workflowRetryPolicyRetryTimeoutValueShouldHaveRightValueWhenBeingSet() {
334+
String expectedName = "TestActivity";
335+
String expectedInput = "TestInput";
336+
String expectedInstanceId = "TestInstanceId";
337+
WorkflowTaskRetryPolicy retryPolicy = WorkflowTaskRetryPolicy.newBuilder()
338+
.setMaxNumberOfAttempts(1)
339+
.setFirstRetryInterval(Duration.ofSeconds(10))
340+
.setRetryTimeout(Duration.ofSeconds(10))
341+
.build();
342+
WorkflowTaskOptions executionOptions = new WorkflowTaskOptions(retryPolicy);
343+
ArgumentCaptor<TaskOptions> captor = ArgumentCaptor.forClass(TaskOptions.class);
344+
345+
context.callChildWorkflow(expectedName, expectedInput, expectedInstanceId, executionOptions, String.class);
346+
347+
verify(mockInnerContext, times(1))
348+
.callSubOrchestrator(
349+
eq(expectedName),
350+
eq(expectedInput),
351+
eq(expectedInstanceId),
352+
captor.capture(),
353+
eq(String.class)
354+
);
355+
356+
TaskOptions taskOptions = captor.getValue();
357+
358+
assertEquals(Duration.ofSeconds(10), taskOptions.getRetryPolicy().getRetryTimeout());
359+
}
360+
361+
@Test
362+
public void workflowRetryPolicyRetryThrowIllegalArgumentWhenNullRetryTimeoutIsSet() {
363+
assertThrows(IllegalArgumentException.class, () ->
364+
WorkflowTaskRetryPolicy.newBuilder()
365+
.setMaxNumberOfAttempts(1)
366+
.setFirstRetryInterval(Duration.ofSeconds(10))
367+
.setRetryTimeout(null)
368+
.build());
369+
}
370+
371+
@Test
372+
public void workflowRetryPolicyRetryThrowIllegalArgumentWhenRetryTimeoutIsLessThanMaxRetryInterval() {
373+
assertThrows(IllegalArgumentException.class, () -> WorkflowTaskRetryPolicy.newBuilder()
374+
.setMaxNumberOfAttempts(1)
375+
.setFirstRetryInterval(Duration.ofSeconds(10))
376+
.setRetryTimeout(Duration.ofSeconds(9))
377+
.build());
378+
}
330379
}

0 commit comments

Comments
 (0)