Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Comments

Example for tasks interaction#567

Open
antmendoza wants to merge 20 commits intotemporalio:main from
antmendoza:taskinteraction_
Open

Example for tasks interaction #567
antmendoza wants to merge 20 commits intotemporalio:main from
antmendoza:taskinteraction_

Conversation

@antmendoza
Copy link
Member

@antmendoza antmendoza commented Dec 26, 2023

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

  1. Closes

  2. How was this tested:

  1. Any docs updates needed?

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

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

Copy link
Member Author

no hurry @tsurdilo

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

@antmendoza antmendoza Jan 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

public class ListOpenTasks {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

@antmendoza antmendoza Jan 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member Author

@antmendoza antmendoza Jan 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

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

Copy link
Member Author

Hi @tsurdilo this is on me, I have to remove the getOpenTask method
tsurdilo reacted with thumbs up emoji

@antmendoza antmendoza marked this pull request as draft March 20, 2024 16:42
antmendoza and others added 16 commits April 30, 2024 08:00 
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] <support@github.com>
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] <support@github.com>
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] <support@github.com>
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] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Tihomir Surdilovic <tihomir@temporal.io>
* Add KeyWordList type to typed SA sample
Signed-off-by: Tihomir Surdilovic <tihomir@temporal.io>
* formatting
Signed-off-by: Tihomir Surdilovic <tihomir@temporal.io>
---------
Signed-off-by: Tihomir Surdilovic <tihomir@temporal.io>
Copy link

CLAassistant commented Apr 30, 2024 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@antmendoza antmendoza marked this pull request as ready for review April 30, 2024 06:16
Copy link
Member Author

This is ready to review @mfateev @tsurdilo

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

Copy link
Contributor

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

Copy link
Contributor

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?

Copy link
Contributor

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

Copy link
Member Author

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?

👍

Copy link
Member Author

antmendoza commented May 29, 2024 •
edited
Loading

@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

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

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Member Author

@antmendoza antmendoza Jun 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we want system outs here?

Copy link
Member Author

@antmendoza antmendoza Jun 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


initTaskList(inputPendingTasks);

Workflow.await(() -> Workflow.getInfo().isContinueAsNewSuggested());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

@antmendoza antmendoza Jun 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tsurdilo tsurdilo tsurdilo approved these changes

@mfateev mfateev Awaiting requested review from mfateev

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /