Example for tasks interaction #567

antmendoza · 2023-12-26T10:03:26Z

What was changed

Why?

Some users look into Temporal as an alternative to BPM tools, like jBPM or Camunda. One common question is how to deal with HumanTask. This example shows a basic implementation to track task state

Checklist

Closes
How was this tested:

Any docs updates needed?

tsurdilo · 2023-12-30T04:29:43Z

hey, sorry still reviewing. ty for doing this sample!

antmendoza · 2024-01-01T09:03:53Z

no hurry @tsurdilo

core/src/main/java/io/temporal/samples/taskinteraction/Task.java

core/src/main/java/io/temporal/samples/taskinteraction/TaskService.java

core/src/main/java/io/temporal/samples/taskinteraction/Task.java

core/src/main/java/io/temporal/samples/taskinteraction/TaskService.java

core/src/main/java/io/temporal/samples/taskinteraction/TaskWorkflowImpl.java

mfateev · 2023-12-31T18:36:56Z

core/src/main/java/io/temporal/samples/taskinteraction/client/UpdateTask.java

+import io.temporal.serviceclient.WorkflowServiceStubs;
+import java.util.Arrays;
+import java.util.List;
+


I think it is confusing to require WorkflowID to complete tasks as they are usually stored in a separate system.
The task token contains the workflow ID already. So I would change the TaskActivityImpl to print the task token and then the UpdateTask to get the token as an argument. The UpdateTask would parse out the workflow id out of the token and signal the appropriate workflow.

right now only the "task token" is required to update / complete the task

https://github.com/temporalio/samples-java/pull/567/files/a92dae84caf9da16e0f5e9874e842d725706339d#diff-b73120e3efff34534d80195613afc83e29b8e8049cb9e796fdeb13147859addbR30

https://github.com/temporalio/samples-java/pull/567/files/a92dae84caf9da16e0f5e9874e842d725706339d#diff-89e67e1da37a2cdfafecf424ebbae1cd65645bdf244e414e802dafd2fdf5208aR47

mfateev · 2023-12-31T18:38:04Z

core/src/main/java/io/temporal/samples/taskinteraction/client/ListOpenTasks.java

+import io.temporal.serviceclient.WorkflowServiceStubs;
+import java.util.List;
+
+public class ListOpenTasks {


I think this is confusing as this list tasks of a specific workflow. In real systems tasks assigned to a specific user would be listed. I would remove the whole list-related functionality to avoid such confusion.

I agree, but also I think it could be useful to show the workflow execution internal state. Do you think we can expose this information in any other way or should I just remove it?

ListOpenTasks removed,

I am maintaining the method getOpenTask to query the internal state and complete the task automatically from UpdateTask

I can remove the method getOpenTask, and have the dev running the example passing the task id to UpdateTask , instead of getting the taskid from the workflow execution using getOpenTask

tsurdilo · 2024-02-12T00:41:29Z

hi, is there any outstanding things to be done on this pr? thanks

antmendoza · 2024-02-12T11:56:41Z

Hi @tsurdilo this is on me, I have to remove the getOpenTask method

… in workflow code

Bumps [org.mockito:mockito-core](https://github.com/mockito/mockito) from 5.8.0 to 5.11.0. - [Release notes](https://github.com/mockito/mockito/releases) - [Commits](mockito/mockito@v5.8.0...v5.11.0) --- updated-dependencies: - dependency-name: org.mockito:mockito-core dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Bumps [ch.qos.logback:logback-classic](https://github.com/qos-ch/logback) from 1.5.0 to 1.5.3. - [Commits](qos-ch/logback@v_1.5.0...v_1.5.3) --- updated-dependencies: - dependency-name: ch.qos.logback:logback-classic 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>

…lio#601) Bumps [com.fasterxml.jackson:jackson-bom](https://github.com/FasterXML/jackson-bom) from 2.16.1 to 2.17.0. - [Commits](FasterXML/jackson-bom@jackson-bom-2.16.1...jackson-bom-2.17.0) --- updated-dependencies: - dependency-name: com.fasterxml.jackson:jackson-bom dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

…mporalio#602) Bumps [com.google.errorprone:error_prone_core](https://github.com/google/error-prone) from 2.25.0 to 2.26.1. - [Release notes](https://github.com/google/error-prone/releases) - [Commits](google/error-prone@v2.25.0...v2.26.1) --- updated-dependencies: - dependency-name: com.google.errorprone:error_prone_core dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Signed-off-by: Tihomir Surdilovic <[email protected]>

* Add KeyWordList type to typed SA sample Signed-off-by: Tihomir Surdilovic <[email protected]> * formatting Signed-off-by: Tihomir Surdilovic <[email protected]> --------- Signed-off-by: Tihomir Surdilovic <[email protected]>

CLAassistant · 2024-04-30T06:10:14Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ tsurdilo
❌ dependabot[bot]
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

antmendoza · 2024-04-30T06:20:40Z

This is ready to review @mfateev @tsurdilo

I have created a new workflow that manages the task lifecycle with which the initial workflow interacts.

joshmsmith · 2024-05-24T16:01:10Z

I like it, this is a nice sample to have.

tsurdilo · 2024-05-26T03:16:20Z

Had a bit of a hard time following example code as in the whole time was thinking this would be good to have as a reusable interceptor that can be just applied to any workflow. Also would be nice to incorporate visibility into this as to
be able to know how many workflows are running that have pending tasks and at which stage of approval process they are.
wdyt?

tsurdilo · 2024-05-26T03:17:21Z

Approving this as think its good sample, just see previous comment about it being bit hard to understand / follow.

antmendoza · 2024-05-26T08:11:27Z

Thanks @tsurdilo

just see previous comment about it being bit hard to understand / follow.

Let me see if I can refactor it / make it simpler

Also would be nice to incorporate visibility into this as to
be able to know how many workflows are running that have pending tasks and at which stage of approval process they are.
wdyt?

👍

antmendoza · 2024-05-29T08:30:57Z

@tsurdilo I have refactored the code and added some comments

This is the commit b2a4f67

let me know if you find it easier to follow now
thanks

tsurdilo · 2024-06-19T15:33:22Z

core/src/main/java/io/temporal/samples/taskinteraction/WorkflowWithTasksImpl.java

+
+    logger.info("Awaiting for the two tasks to complete");
+    // Block execution until both tasks complete
+    Promise.allOf(Arrays.asList(task1, task2)).get();


i think here you need to loop through promises again to check if any failed and if not call .get on each (to make sure they both actually compelted)

I get your point and i can add it, but I don't think any of these promises can fail with the current implementation

https://github.com/temporalio/samples-java/pull/567/files#diff-89e67e1da37a2cdfafecf424ebbae1cd65645bdf244e414e802dafd2fdf5208aR88

tsurdilo · 2024-06-19T15:33:59Z

core/src/main/java/io/temporal/samples/taskinteraction/client/CompleteTask.java

+
+    Thread.sleep(200);
+    final List<Task> pendingTask = getPendingTask(workflowTaskManager);
+    System.out.println("Pending task " + pendingTask);


do we want system outs here?

This is the client completing task , and this shows pending tasks.

tsurdilo · 2024-06-19T15:58:43Z

core/src/main/java/io/temporal/samples/taskinteraction/WorkflowTaskManagerImpl.java

+
+    initTaskList(inputPendingTasks);
+
+    Workflow.await(() -> Workflow.getInfo().isContinueAsNewSuggested());


maybe im missing something here but if all tasks in task list complete
before continueAsNewSuggested triggers this exec will running

i wonder if this can be changed to like "if we did not receive any new tasks for duration X" maybe instead?
reason is that cost of running execs on cloud so users might wanna complete this exec and use signalWithStart instead when new tasks come in? maybe im missing something so let me know

I can add a condition so if there are no pending tasks the workflow returns/closes, WDYT @tsurdilo

antmendoza requested review from tsurdilo and a team as code owners December 26, 2023 10:03

mfateev reviewed Jan 2, 2024

View reviewed changes

antmendoza requested a review from mfateev January 9, 2024 13:50

antmendoza marked this pull request as draft March 20, 2024 16:42

antmendoza and others added 16 commits April 30, 2024 08:00

task interaction

989b9e1

using UpperCamelCase for the enum name and lowerCamelCase for values.

54fcd36

changed from synchronizedMap to Map, as synchronization is not needed…

bb771c3

… in workflow code

moved taskToken mothod to a class

f9b0810

removed executeTaskAsync method

8f3e298

stored task data as a serialized type, instead of object

08ecfa0

removed ListOpenTasks

3b83278

wip

6a1dbea

update sdk version to 1.23.1 (temporalio#608)

31df9e4

Signed-off-by: Tihomir Surdilovic <[email protected]>

Add KeyWordList type to typed SA sample (temporalio#607)

a87db56

* Add KeyWordList type to typed SA sample Signed-off-by: Tihomir Surdilovic <[email protected]> * formatting Signed-off-by: Tihomir Surdilovic <[email protected]> --------- Signed-off-by: Tihomir Surdilovic <[email protected]>

Example for tasks interaction

677f8b0

Merge remote-tracking branch 'core/main' into taskinteraction_

9fd4389

antmendoza force-pushed the taskinteraction_ branch from 18e3c12 to 9fd4389 Compare April 30, 2024 06:10

fix commented code

c8742b1

antmendoza marked this pull request as ready for review April 30, 2024 06:16

fix readme

0043cc8

tsurdilo approved these changes May 26, 2024

View reviewed changes

Merge remote-tracking branch 'core/main' into taskinteraction_

cd5b7c9

code-refactor

b2a4f67

antmendoza force-pushed the taskinteraction_ branch from 97e7a38 to b2a4f67 Compare May 29, 2024 08:35

antmendoza requested a review from tsurdilo May 30, 2024 19:01

antmendoza requested review from tsurdilo and removed request for tsurdilo June 9, 2024 16:24

tsurdilo reviewed Jun 19, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Example for tasks interaction #567

Example for tasks interaction #567

antmendoza commented Dec 26, 2023

tsurdilo commented Dec 30, 2023

antmendoza commented Jan 1, 2024

mfateev Dec 31, 2023

antmendoza Jan 3, 2024 •

edited

Loading

mfateev Dec 31, 2023

antmendoza Jan 3, 2024

antmendoza Jan 9, 2024

tsurdilo commented Feb 12, 2024

antmendoza commented Feb 12, 2024

CLAassistant commented Apr 30, 2024 •

edited

Loading

antmendoza commented Apr 30, 2024

joshmsmith commented May 24, 2024

tsurdilo commented May 26, 2024

tsurdilo commented May 26, 2024

antmendoza commented May 26, 2024

antmendoza commented May 29, 2024 •

edited

Loading

tsurdilo Jun 19, 2024

antmendoza Jun 19, 2024

tsurdilo Jun 19, 2024

antmendoza Jun 19, 2024

tsurdilo Jun 19, 2024

antmendoza Jun 19, 2024


		initTaskList(inputPendingTasks);

		Workflow.await(() -> Workflow.getInfo().isContinueAsNewSuggested());

Example for tasks interaction #567

Are you sure you want to change the base?

Example for tasks interaction #567

Conversation

antmendoza commented Dec 26, 2023

What was changed

Why?

Checklist

tsurdilo commented Dec 30, 2023

antmendoza commented Jan 1, 2024

Choose a reason for hiding this comment

antmendoza Jan 3, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tsurdilo commented Feb 12, 2024

antmendoza commented Feb 12, 2024

CLAassistant commented Apr 30, 2024 • edited Loading

antmendoza commented Apr 30, 2024

joshmsmith commented May 24, 2024

tsurdilo commented May 26, 2024

tsurdilo commented May 26, 2024

antmendoza commented May 26, 2024

antmendoza commented May 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

antmendoza Jan 3, 2024 •

edited

Loading

CLAassistant commented Apr 30, 2024 •

edited

Loading

antmendoza commented May 29, 2024 •

edited

Loading