-
Notifications
You must be signed in to change notification settings - Fork 207
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
Workflow Implementation - Continues... #880
Conversation
Signed-off-by: LionTao <taojiachun980831@163.com> Signed-off-by: LionTao <taojiachun980831@163.com> Co-authored-by: Mukundan Sundararajan <65565396+mukundansundar@users.noreply.github.com> Co-authored-by: Artur Souza <artursouza.ms@outlook.com> Signed-off-by: Mahmut Canga <cangamahmut@gmail.com>
Bumps [codecov/codecov-action](https://github.com/codecov/codecov-action) from 3.1.0 to 3.1.1. - [Release notes](https://github.com/codecov/codecov-action/releases) - [Changelog](https://github.com/codecov/codecov-action/blob/master/CHANGELOG.md) - [Commits](codecov/codecov-action@v3.1.0...v3.1.1) --- updated-dependencies: - dependency-name: codecov/codecov-action dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Artur Souza <artursouza.ms@outlook.com> Signed-off-by: Mahmut Canga <cangamahmut@gmail.com>
Signed-off-by: Mahmut Canga <cangamahmut@gmail.com>
Signed-off-by: Artur Souza <asouza.pro@gmail.com> Signed-off-by: Mahmut Canga <cangamahmut@gmail.com>
Signed-off-by: Artur Souza <asouza.pro@gmail.com> Signed-off-by: Mahmut Canga <cangamahmut@gmail.com>
Signed-off-by: Artur Souza <asouza.pro@gmail.com> Signed-off-by: Mahmut Canga <cangamahmut@gmail.com>
Signed-off-by: Artur Souza <asouza.pro@gmail.com> Signed-off-by: Mahmut Canga <cangamahmut@gmail.com>
* Bump from spring boot 2.3.5.RELEASE to 2.7.8 Signed-off-by: Sergio <champel@gmail.com> (cherry picked from commit 9152c91) * Ensure old versions of spring boot are still compatible Signed-off-by: Sergio <champel@gmail.com> --------- Signed-off-by: champel <champel@gmail.com> Signed-off-by: Sergio <champel@gmail.com> Signed-off-by: Mahmut Canga <cangamahmut@gmail.com>
* Bump from reactor 2.3.5.RELEASE to 2.7.8 Signed-off-by: Sergio <champel@gmail.com> * Simplification Signed-off-by: Sergio <champel@gmail.com> --------- Signed-off-by: Sergio <champel@gmail.com> Signed-off-by: Mahmut Canga <cangamahmut@gmail.com>
Signed-off-by: Artur Souza <asouza.pro@gmail.com> Signed-off-by: Mahmut Canga <cangamahmut@gmail.com>
Signed-off-by: Mahmut Canga <cangamahmut@gmail.com>
* add grpc subscriber Signed-off-by: MregXN <mregxn@gmail.com> * modify README.md Signed-off-by: MregXN <mregxn@gmail.com> * modify README.md in examples Signed-off-by: MregXN <mregxn@gmail.com> * Modify DaprApplication to support examples where protocol is not specified. Signed-off-by: MregXN <mregxn@gmail.com> * modify formatter to pass checkstyle Signed-off-by: MregXN <mregxn@gmail.com> * Update springboot to latest minor.patch version. (dapr#826) Signed-off-by: MregXN <mregxn@gmail.com> * Use runtime 1.10.0-rc.X and CLI 1.10.0-rc.X (dapr#827) Signed-off-by: Artur Souza <asouza.pro@gmail.com> Signed-off-by: MregXN <mregxn@gmail.com> * Upgrade the version to 1.9.0-SNAPSHOT (dapr#829) Signed-off-by: Artur Souza <asouza.pro@gmail.com> Signed-off-by: MregXN <mregxn@gmail.com> * Generate updated javadocs for 1.8.0 (dapr#836) Signed-off-by: Artur Souza <asouza.pro@gmail.com> Signed-off-by: MregXN <mregxn@gmail.com> * Update Dapr runtime and CLI to 1.10. (dapr#837) Signed-off-by: Artur Souza <asouza.pro@gmail.com> Signed-off-by: MregXN <mregxn@gmail.com> * Inject autoconfiguration in the Spring Boot 3 style (dapr#831) * Bump from spring boot 2.3.5.RELEASE to 2.7.8 Signed-off-by: Sergio <champel@gmail.com> (cherry picked from commit 9152c91) * Ensure old versions of spring boot are still compatible Signed-off-by: Sergio <champel@gmail.com> --------- Signed-off-by: champel <champel@gmail.com> Signed-off-by: Sergio <champel@gmail.com> Signed-off-by: MregXN <mregxn@gmail.com> * Bump from reactor 2.3.5.RELEASE to 2.7.8 (dapr#830) * Bump from reactor 2.3.5.RELEASE to 2.7.8 Signed-off-by: Sergio <champel@gmail.com> * Simplification Signed-off-by: Sergio <champel@gmail.com> --------- Signed-off-by: Sergio <champel@gmail.com> Signed-off-by: MregXN <mregxn@gmail.com> * rerun checks Signed-off-by: MregXN <mregxn@gmail.com> * modify the way of grpc server starts Signed-off-by: MregXN <mregxn@gmail.com> * modify README Signed-off-by: MregXN <mregxn@gmail.com> * Update pom.xml Signed-off-by: MregXN <46479059+MregXN@users.noreply.github.com> --------- Signed-off-by: MregXN <mregxn@gmail.com> Signed-off-by: Artur Souza <asouza.pro@gmail.com> Signed-off-by: champel <champel@gmail.com> Signed-off-by: Sergio <champel@gmail.com> Signed-off-by: MregXN <46479059+MregXN@users.noreply.github.com> Co-authored-by: Artur Souza <artursouza.ms@outlook.com> Co-authored-by: champel <champel@gmail.com> Co-authored-by: Mukundan Sundararajan <65565396+mukundansundar@users.noreply.github.com> Signed-off-by: Mahmut Canga <cangamahmut@gmail.com>
Signed-off-by: Mukundan Sundararajan <65565396+mukundansundar@users.noreply.github.com> Signed-off-by: Mahmut Canga <cangamahmut@gmail.com>
Bumps [codecov/codecov-action](https://github.com/codecov/codecov-action) from 3.1.1 to 3.1.4. - [Release notes](https://github.com/codecov/codecov-action/releases) - [Changelog](https://github.com/codecov/codecov-action/blob/main/CHANGELOG.md) - [Commits](codecov/codecov-action@v3.1.1...v3.1.4) --- updated-dependencies: - dependency-name: codecov/codecov-action 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> Signed-off-by: Mahmut Canga <cangamahmut@gmail.com>
Signed-off-by: Artur Souza <asouza.pro@gmail.com> Signed-off-by: Mahmut Canga <cangamahmut@gmail.com>
Signed-off-by: Bill DeRusha <billderusha@microsoft.com> Signed-off-by: Mahmut Canga <cangamahmut@gmail.com>
Signed-off-by: Hannah Kennedy <hakenned@microsoft.com> Signed-off-by: Mahmut Canga <cangamahmut@gmail.com>
Signed-off-by: Hannah Kennedy <hakenned@microsoft.com> Signed-off-by: Mahmut Canga <cangamahmut@gmail.com>
Signed-off-by: Bill DeRusha <444835+bderusha@users.noreply.github.com> Signed-off-by: Mahmut Canga <cangamahmut@gmail.com>
Signed-off-by: Bill DeRusha <444835+bderusha@users.noreply.github.com> Signed-off-by: Mahmut Canga <cangamahmut@gmail.com>
Signed-off-by: Bill DeRusha <444835+bderusha@users.noreply.github.com> Signed-off-by: Mahmut Canga <cangamahmut@gmail.com>
Signed-off-by: Bill DeRusha <444835+bderusha@users.noreply.github.com> Signed-off-by: Mahmut Canga <cangamahmut@gmail.com>
Signed-off-by: Bill DeRusha <444835+bderusha@users.noreply.github.com> Signed-off-by: Mahmut Canga <cangamahmut@gmail.com>
Signed-off-by: Bill DeRusha <444835+bderusha@users.noreply.github.com> Signed-off-by: Mahmut Canga <cangamahmut@gmail.com>
* addiny getInstanceMetadata, waitForInstanceStart and waitForInstanceCompletion implementation --------- Co-authored-by: aymanmahmoud_microsoft <aymanmahmoud@microsoft.com> Signed-off-by: Aymand Mahmoud <aymanmahmoud@microsoft.com> Signed-off-by: Mahmut Canga <cangamahmut@gmail.com>
Signed-off-by: Mahmut Canga <cangamahmut@gmail.com>
Signed-off-by: Mahmut Canga <cangamahmut@gmail.com>
Co-authored-by: Julio Rezende <jsilvarezend@microsoft.com> Signed-off-by: Mahmut Canga <cangamahmut@gmail.com> Signed-off-by: Julio Rezende <jsilvarezend@microsoft.com>
Co-authored-by: Julio Rezende <jsilvarezend@microsoft.com> Signed-off-by: Mahmut Canga <cangamahmut@gmail.com> Signed-off-by: Julio Rezende <jsilvarezend@microsoft.com>
Signed-off-by: Mahmut Canga <cangamahmut@gmail.com>
Signed-off-by: Mahmut Canga <cangamahmut@gmail.com>
Signed-off-by: Mahmut Canga <cangamahmut@gmail.com>
Signed-off-by: Mahmut Canga <cangamahmut@gmail.com>
Signed-off-by: Mahmut Canga <cangamahmut@gmail.com>
Signed-off-by: Mahmut Canga <cangamahmut@gmail.com>
Signed-off-by: Mahmut Canga <cangamahmut@gmail.com>
Signed-off-by: Mahmut Canga <cangamahmut@gmail.com>
Signed-off-by: Mahmut Canga <cangamahmut@gmail.com>
Signed-off-by: Mahmut Canga <cangamahmut@gmail.com>
Signed-off-by: Mahmut Canga <cangamahmut@gmail.com>
@skyao I believe I have covered all the comments in my latest commits. |
Fair points @skyao 👍🏼 The concept of workflow and running activities as a workflow is not a common paradigm that we could see in most applications. However, when we consider the choreography vs orchestration discussion in microservices architecture, workflows are closer to orchestration. An orchestrator by design should be able to provide an instance of its workflow running. Also, it should provide a client for an external entities (another program, the developer etc.) to operate on the orchestration. These two concerns are covered by On the other hand, an orchestrator needs tasks to complete the business objective. In our case, these are covered by In this context, someone looking at dapr workflows for the first time needs a clear understanding of which path they are choosing orchestration over choreography. The example project provides the implementation details but the documentation should touch the above concepts in detail. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! Thank for your contribution @macromania !
I have competed my review and confirmed that all my comments have been updated.
@artursouza @mukundansundar please help to have a look at this PR and if OK please help to merge it.
@artursouza @mukundansundar please help to merge this PR. |
I need to confirm the design of dapr workflow java sdk: our dapr workflow java sdk will access backend of durable task directly and bypass dapr's sidecar (including dapr api layer and all workflow components). To confirm this design, I list a lot of our implementation:
service Dapr {
// Starts a new instance of a workflow
rpc StartWorkflowAlpha1 (StartWorkflowRequest) returns (StartWorkflowResponse) {}
// Gets details about a started workflow instance
rpc GetWorkflowAlpha1 (GetWorkflowRequest) returns (GetWorkflowResponse) {}
// Purge Workflow
rpc PurgeWorkflowAlpha1 (PurgeWorkflowRequest) returns (google.protobuf.Empty) {}
// Terminates a running workflow instance
rpc TerminateWorkflowAlpha1 (TerminateWorkflowRequest) returns (google.protobuf.Empty) {}
// Pauses a running workflow instance
rpc PauseWorkflowAlpha1 (PauseWorkflowRequest) returns (google.protobuf.Empty) {}
// Resumes a paused workflow instance
rpc ResumeWorkflowAlpha1 (ResumeWorkflowRequest) returns (google.protobuf.Empty) {}
// Raise an event to a running workflow instance
rpc RaiseEventWorkflowAlpha1 (RaiseEventWorkflowRequest) returns (google.protobuf.Empty) {}
}
import com.microsoft.durabletask.DurableTaskClient;
public class DaprWorkflowClient implements AutoCloseable {
DurableTaskClient innerClient;
ManagedChannel grpcChannel;
public <T extends Workflow> String scheduleNewWorkflow(Class<T> clazz) {
return this.innerClient.scheduleNewOrchestrationInstance(clazz.getCanonicalName());
} And the method of dapr workflow api like StartWorkflowAlpha1/GetWorkflowAlpha1 are never called in the two PR.
class DaprWorkflowClient:
"""Defines client operations for managing Dapr Workflow instances.
This is an alternative to the general purpose Dapr client. It uses a gRPC connection to send
commands directly to the workflow engine, bypassing the Dapr API layer.
This client is intended to be used by workflow application, not by general purpose
application.
""" So, with current design of java workflow sdk, we only provides support for microsoft durabletask and the implementation is hard code. |
Please refer this PR for python-sdk, where management APIs are used: dapr/python-sdk#554 This PR for dotnet: dapr/dotnet-sdk#1003 |
sdk-workflows/src/main/java/io/dapr/workflows/client/DaprWorkflowClient.java
Outdated
Show resolved
Hide resolved
sdk-workflows/src/main/java/io/dapr/workflows/client/WorkflowFailureDetails.java
Outdated
Show resolved
Hide resolved
* data provided to it by the orchestrator. | ||
* @return any serializable value to be returned to the calling orchestrator. | ||
*/ | ||
Object run(WorkflowActivityContext ctx); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use generic and the serializer interface to return the actual object. See how it is done for DaprClient
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@artursouza this run() method defined in WorkflowActivity doesn't have a input parameter, we need to call ctx.getInput() to get the input, like this:
public Object run(WorkflowActivityContext ctx) {
OrderPayload order = ctx.getInput(OrderPayload.class);
logger.info("Requesting approval for order: {}", order);
return ApprovalResult.Approved;
}
I suggest to move this input parameter from ctx to run() method:
public interface WorkflowActivity<Input, Output>{
Output run(WorkflowActivityContext ctx, Input input);
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exactly. Also, use TypeRef from the main SDK. See how we do it for Actors and state store.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, make this as a separate PR. I will make the other changes and merge this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have permissions to edit files in this PR. Please, address the other comments so I can merge this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just pushed the changes, thanks for your review @artursouza 👍🏼
Signed-off-by: Mahmut Canga <cangamahmut@gmail.com>
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #880 +/- ##
============================================
- Coverage 76.94% 76.90% -0.05%
- Complexity 1326 1395 +69
============================================
Files 126 132 +6
Lines 4091 4230 +139
Branches 484 493 +9
============================================
+ Hits 3148 3253 +105
- Misses 696 720 +24
- Partials 247 257 +10
☔ View full report in Codecov by Sentry. |
Description
This PR continues the Workflow implementation on top of #857. This PR will be rebased once #857 is merged to main. So, about 20 of the files in this PR would be already reviewed and merged to main 👀
In the meantime, please feel free to review and provide feedback 😅
As seen from the implementation, it involves abstraction of Durable Task Framework in most places. This means some of the functionality are not provided in this PR as they also lack implementation support at Durable Task Framework.
Management API
As mentioned in microsoft/durabletask-java#142,
queryInstances
andpurgeInstances
are not supported due to limitations at Dapr datastore.Authoring API
Testing
We have provided unit test coverage with mocks for all the SDK methods. Also, we have expanded the example provided in #857 by adding all of the Authoring and Management API calls. We used the example as a mean to test the integrity of the code and also understand if the developer experience is satisfactory.
When the example is run, following output is expected from the
DemoWorkflow
.The above logs show that:
TimedOutEvent
and eventually time outTestEvent
and show the event is receivedallOf
anyOf
Activity
Activity
Activity
ChildWorkflow
Also, when you run the example to the end, you will be showcased:
purgeInstance
terminate
restart
Issue reference
#839
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: