Workflow Authoring #559

DeepanshuA · 2023-04-25T10:41:06Z

Description

This PR implements Workflow Authoring part.

Issue reference

We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.

Please reference the issue this PR is related to: #542

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

Code compiles correctly
Created/updated tests
Extended the documentation

Signed-off-by: Deepanshu Agarwal <deepanshu.agarwal1984@gmail.com>

…horing

Signed-off-by: Deepanshu Agarwal <deepanshu.agarwal1984@gmail.com>

codecov · 2023-04-25T16:28:04Z

Codecov Report

Merging #559 (a9be483) into master (5219da0) will increase coverage by 0.01%.
The diff coverage is 89.91%.

@@            Coverage Diff             @@
##           master     #559      +/-   ##
==========================================
+ Coverage   89.59%   89.60%   +0.01%     
==========================================
  Files          62       72      +10     
  Lines        2978     3310     +332     
==========================================
+ Hits         2668     2966     +298     
- Misses        310      344      +34

Impacted Files	Coverage Δ
...ext-workflow/dapr/ext/workflow/workflow_runtime.py	`76.92% <76.92%> (ø)`
...orkflow/dapr/ext/workflow/dapr_workflow_context.py	`77.77% <77.77%> (ø)`
...ext-workflow/dapr/ext/workflow/workflow_context.py	`79.48% <79.48%> (ø)`
...r-ext-workflow/dapr/ext/workflow/workflow_state.py	`83.87% <83.87%> (ø)`
...t/dapr-ext-workflow/tests/test_workflow_runtime.py	`92.85% <92.85%> (ø)`
...r-ext-workflow/tests/test_dapr_workflow_context.py	`94.11% <94.11%> (ø)`
...low/dapr/ext/workflow/workflow_activity_context.py	`94.73% <94.73%> (ø)`
...workflow/dapr/ext/workflow/dapr_workflow_client.py	`97.43% <97.43%> (ø)`
...xt/dapr-ext-workflow/tests/test_workflow_client.py	`98.27% <98.27%> (ø)`
...xt/dapr-ext-workflow/dapr/ext/workflow/__init__.py	`100.00% <100.00%> (ø)`
... and 1 more

DeepanshuA · 2023-04-25T17:51:14Z

Removing DRAFT label from PR, but I still need to check-in test cases for it, add remaining client APIs and also see how I can remove get-pip.py.

Signed-off-by: Deepanshu Agarwal <deepanshu.agarwal1984@gmail.com>

berndverst · 2023-04-27T21:33:51Z

examples/demo_workflow/requirements.txt

@@ -0,0 +1 @@
+dapr-ext-workflow==1.5.0


Suggested change

dapr-ext-workflow==1.5.0

dapr-ext-workflow-dev==0.0.1-rc.1

Where did you come up with this version? :D That will not be the version number we are using.

What version would you like to launch the SDK with? I would assume 0.1.0?

In the master branch we always import our development packages -- so use the -dev one. In the release branches we then remove the -dev suffix to refer to the stable packages. The -dev one is the builds from master that we constantly update.

Yeah, you are correct. I don't know how I wrote this number here :)
Will fix it.

Updated to 0.0.1rc1 as per version.py of dapr-ext-workflow

…horing

Signed-off-by: Deepanshu Agarwal <deepanshu.agarwal1984@gmail.com>

ext/dapr-ext-workflow/setup.cfg

berndverst · 2023-04-28T20:07:47Z

examples/demo_workflow/app.py

+daprOptions.register_activity(hello_act)
+daprOptions.run()
+
+client = WorkflowClient("localhost","4001")


What is this port 4001? Where is this configured? Why this port?

@DeepanshuA let's remove the explicit port configuration from this example. The default value of DAPR_GRPC_PORT will be what most users want.

Updated to use DAPR_GRPC_PORT. If that is not found, then "4001" is used.
Used "4001" due to https://github.com/microsoft/durabletask-python/blob/dcf205831124d82ca3886e5dba26d8c7a9ac8d73/durabletask/internal/shared.py#L18

Signed-off-by: Deepanshu Agarwal <deepanshu.agarwal1984@gmail.com>

cgillum · 2023-05-09T05:02:32Z

examples/demo_workflow/app.py

+daprOptions.register_activity(hello_act)
+daprOptions.run()
+
+client = WorkflowClient("localhost","4001")


@DeepanshuA let's remove the explicit port configuration from this example. The default value of DAPR_GRPC_PORT will be what most users want.

examples/demo_workflow/app.py

cgillum · 2023-05-09T05:06:07Z

ext/dapr-ext-workflow/dapr/ext/workflow/dapr_workflow_context.py

@@ -0,0 +1,47 @@
+from typing import Callable, TypeVar, Union


Let's add the copyright header to all these files you're adding.

ext/dapr-ext-workflow/dapr/ext/workflow/dapr_workflow_context.py

ext/dapr-ext-workflow/dapr/ext/workflow/workflow_activity_context.py

cgillum · 2023-05-09T05:11:20Z

ext/dapr-ext-workflow/dapr/ext/workflow/workflow_client.py

+            host = settings.DAPR_RUNTIME_HOST
+        if port is None:
+            port = settings.DAPR_GRPC_PORT
+        address = f"{host}:{port}"


Let's raise an error if either host or port is still None even after running through this logic.

Updated to set "host" to localhost, if DAPR_RUNTIME_HOST is not found.
Updated to raise an error, if DAPR_GRPC_PORT is not found.

cgillum · 2023-05-09T05:17:35Z

ext/dapr-ext-workflow/setup.cfg

@@ -25,6 +25,7 @@ packages = find_namespace:
 include_package_data = True
 install_requires =
    dapr-dev >= 1.9.0rc1.dev
+    durabletask >= 0.1.0a2


Do we need to update this again after your latest fix to make durabletask-python compatible with 3.7 or does this version already have that support?

Yes it seems you need to release 0.1.0a3 and then update this here. The version with 3.7 compatibility has not been released yet.

0.1.0a2 durabletask-python also has 3.7 support. But, had marked 0.1.0a3 in durabletask-python, as wanted to keep Pypi and source code on github in sync. As, 0.1.0a2 had Pypi support even when 3.7 compliant PR was not merged.
Need to automate the release process in durabletask-python, I think then this mis-match issue will not be there.

…horing

berndverst

In general I'm very confused how WorkflowClient here overlaps with Ryan's PR for workflow management.

I assume in this PR it is more of a WorkflowHost or WorkflowServer which the Dapr sidecar calls into?
If this understanding is correct, can we please avoid calling the class WorkflowClient ?

OrchestratorClient or WorkflowEngineClient or WorkflowEngineCallbackClient or something like that makes more sense to me. I assume this is how the sidecar tells your workflow to execute certain operations like pause, start, resume etc

berndverst · 2023-05-09T23:27:47Z

ext/dapr-ext-workflow/dapr/ext/workflow/workflow_runtime_options.py

+
+
+class WorkflowRuntimeOptions:
+


Please add a doc string. It's not apparent to me what this class is for.

Renamed it to WorkflowRuntime and provided a doc string.

ext/dapr-ext-workflow/dapr/ext/workflow/workflow_context.py

ext/dapr-ext-workflow/dapr/ext/workflow/workflow_client.py

examples/demo_workflow/app.py

berndverst · 2023-05-10T00:07:59Z

Hmm or instead of WorkflowClient it could be WorkflowHostClient WorkflowProviderClient or something like that.

Anything but WorkflowClient :)

Signed-off-by: Deepanshu Agarwal <deepanshu.agarwal1984@gmail.com>

examples/demo_workflow/components/state_redis.yaml

tmacam · 2023-05-16T05:18:58Z

tox.ini

+    ; ./validate.sh demo_actor
+    ./validate.sh demo_workflow
    ./validate.sh invoke-binding
    ./validate.sh grpc_proxying
    ./validate.sh w3c-tracing
    ./validate.sh distributed_lock
-    ./validate.sh configuration
+    ; ./validate.sh configuration


Any reason to leave these commented out?

Yeah, these tests are failing with 1.10.x Dapr version. These should be fixed with 1.11.x version though, So, will remove these commented outs, once 1.11.x can be pinned and validation can run against them.

Signed-off-by: Deepanshu Agarwal <deepanshu.agarwal1984@gmail.com>

DeepanshuA · 2023-05-16T10:54:45Z

@cgillum @berndverst Please re-review this PR. I have tried to address comments and included validate example for workflow as well.
I have commented out demo_actor and configuration from validate example, as they were failing with current Dapr 1.10.7 version. Will include them, once we have 1.11.x pointed here.

cgillum

Just a few additional comments from me. Note that I didn't spend much time on the test-related files since I don't really understand how the mocking works (and I'm assuming it's not something requiring critical review).

cgillum · 2023-05-16T22:31:24Z

examples/demo_workflow/app.py

+from dapr.ext.workflow.workflow_runtime import WorkflowRuntime
+from dapr.ext.workflow.dapr_workflow_client import DaprWorkflowClient
+from dapr.ext.workflow.dapr_workflow_context import DaprWorkflowContext
+from dapr.ext.workflow.workflow_activity_context import WorkflowActivityContext


Do we really need so many modules or could we reduce it down to just one or two? Note that we don't need to organize files the same way we do for .NET. Ideally we could just do something like:

from dapr.ext.workflow import DaprWorkflowClient, DaprWorkflowContext, WorkflowActivityContext, WorkflowRuntime

...or...

import dapr.ext.workflow as wf

And just refer to everything from the wf. prefix, like wf.DaprWorkflowContext, etc.

We could import those into dapr.ext.workflow's init.py exactly as the code about so they could be imported as you mentioned. Agree it would make the existing and future code simpler.

Done. Please check now.

In recent validate examples, somehow import in app.py is failing.
Everything seems working on my local though.
https://github.com/dapr/python-sdk/actions/runs/5020485194/jobs/9001991467#step:17:279

trying to understand that what can be the issue, is somehow dapr-ext-workflow build used in validate examples, not consisting of my init.py changes
@berndverst Any suggestions here?

examples/demo_workflow/app.py

examples/demo_workflow/components/state_redis.yaml

cgillum · 2023-05-16T23:11:21Z

ext/dapr-ext-workflow/dapr/ext/workflow/dapr_workflow_client.py

+        if port is None:
+            port = settings.DAPR_GRPC_PORT
+        if port is None:
+            raise ValueError("Port cannot be empty, please set DAPR_GRPC_PORT environment variable")


If no port is specified, let's default to 50001. This is the documented default port for gRPC.

Actually, in global_settings.py, DAPR_GRPC_PORT is already defined to its default: https://github.com/dapr/python-sdk/blob/master/dapr/conf/global_settings.py#L24
So, it is just an extra check.

Ah, my bad. It might actually be better to remove the check to avoid confusion. Too much defense in depth can be misleading.

examples/demo_workflow/README.md

tmacam · 2023-05-16T21:25:10Z

examples/demo_workflow/README.md

+
+dapr run --app-id orderapp --app-protocol grpc --dapr-grpc-port 4001 --components-path components --placement-host-address localhost:50005 -- python3 app.py
+```
+
+<!-- END_STEP -->


I would move this into a separate section, named "Running this example".

Suggested change

dapr run --app-id orderapp --app-protocol grpc --dapr-grpc-port 4001 --components-path components --placement-host-address localhost:50005 -- python3 app.py

```





## Running this example

To run this example, the following code can be utilized:

```bash

dapr run --app-id orderapp --app-protocol grpc --dapr-grpc-port 4001 --components-path components --placement-host-address localhost:50005 -- python3 app.py

And we need some validation on the execution of this workflow, right?

Actually, app.py has got some required assertions.

Also, added expected_stdout_lines in Readme itself.

examples/demo_workflow/app.py

ext/dapr-ext-workflow/dapr/ext/workflow/dapr_workflow_client.py

ext/dapr-ext-workflow/dapr/ext/workflow/workflow_activity_context.py

Signed-off-by: Deepanshu Agarwal <deepanshu.agarwal1984@gmail.com>

…horing

Signed-off-by: Deepanshu Agarwal <deepanshu.agarwal1984@gmail.com>

examples/demo_workflow/demo_workflow/requirements.txt

Signed-off-by: Bernd Verst <github@bernd.dev>

Signed-off-by: Deepanshu Agarwal <deepanshu.agarwal1984@gmail.com>

berndverst · 2023-05-22T21:24:35Z

Let's see if the ordering works 👀 - that should give placement service plenty of time to initialize

DeepanshuA · 2023-05-22T21:32:01Z

Let's see if the ordering works 👀 - that should give placement service plenty of time to initialize

surprisingly, it still failed - don't know if placement goes in unstable state in-between due to raft election etc.

Signed-off-by: Deepanshu Agarwal <deepanshu.agarwal1984@gmail.com>

berndverst

I verified this locally. I will merge this now - but if we find an issue, please be prepared to create a follow up PR.

Workflow Authoring

c4e9ef7

Signed-off-by: Deepanshu Agarwal <deepanshu.agarwal1984@gmail.com>

DeepanshuA requested review from a team as code owners April 25, 2023 10:41

Add example

487e97f

Signed-off-by: Deepanshu Agarwal <deepanshu.agarwal1984@gmail.com>

DeepanshuA changed the title ~~Workflow Authoring~~ [DRAFT] Workflow Authoring Apr 25, 2023

DeepanshuA added 4 commits April 25, 2023 16:49

Merge branch 'master' of github.com:dapr/python-sdk into workflow_aut…

0733570

…horing

lint

26380ee

Signed-off-by: Deepanshu Agarwal <deepanshu.agarwal1984@gmail.com>

is it wheel fix

eb995b8

Signed-off-by: Deepanshu Agarwal <deepanshu.agarwal1984@gmail.com>

fix lint

cd044d1

Signed-off-by: Deepanshu Agarwal <deepanshu.agarwal1984@gmail.com>

DeepanshuA changed the title ~~[DRAFT] Workflow Authoring~~ Workflow Authoring Apr 25, 2023

DeepanshuA added 2 commits April 27, 2023 21:04

Add tests and client APIs

128a1fa

Signed-off-by: Deepanshu Agarwal <deepanshu.agarwal1984@gmail.com>

lint

9361b30

Signed-off-by: Deepanshu Agarwal <deepanshu.agarwal1984@gmail.com>

berndverst reviewed Apr 27, 2023

View reviewed changes

DeepanshuA added 3 commits April 29, 2023 00:05

Merge branch 'master' of github.com:dapr/python-sdk into workflow_aut…

f82bb8e

…horing

Add dtf python dependency

ffde1df

Signed-off-by: Deepanshu Agarwal <deepanshu.agarwal1984@gmail.com>

correction

8d62488

Signed-off-by: Deepanshu Agarwal <deepanshu.agarwal1984@gmail.com>

berndverst reviewed Apr 28, 2023

View reviewed changes

ext/dapr-ext-workflow/setup.cfg Outdated Show resolved Hide resolved

berndverst reviewed Apr 28, 2023

View reviewed changes

DeepanshuA and others added 5 commits May 2, 2023 15:06

Remove get-pip.py

1be2447

Signed-off-by: Deepanshu Agarwal <deepanshu.agarwal1984@gmail.com>

Update durabletask dependency version

a537862

Signed-off-by: Deepanshu Agarwal <deepanshu.agarwal1984@gmail.com>

Extra line - to be deleted

75ae2bf

Signed-off-by: Deepanshu Agarwal <deepanshu.agarwal1984@gmail.com>

test compatible with 3.7

7a31c66

Signed-off-by: Deepanshu Agarwal <deepanshu.agarwal1984@gmail.com>

Merge branch 'master' into workflow_authoring

325dedb

cgillum reviewed May 9, 2023

View reviewed changes

Merge branch 'master' of github.com:dapr/python-sdk into workflow_aut…

b8bca56

…horing

berndverst reviewed May 10, 2023

View reviewed changes

Incorporate review comments

2f81812

Signed-off-by: Deepanshu Agarwal <deepanshu.agarwal1984@gmail.com>

DeepanshuA added 2 commits May 12, 2023 08:46

Remove demo actor temporarily

74dbc9f

Signed-off-by: Deepanshu Agarwal <deepanshu.agarwal1984@gmail.com>

Include raise event test and assertions

bf69dea

Signed-off-by: Deepanshu Agarwal <deepanshu.agarwal1984@gmail.com>

tmacam reviewed May 16, 2023

View reviewed changes

examples/demo_workflow/components/state_redis.yaml Show resolved Hide resolved

tmacam reviewed May 16, 2023

View reviewed changes

Rename

0968b8e

Signed-off-by: Deepanshu Agarwal <deepanshu.agarwal1984@gmail.com>

cgillum reviewed May 16, 2023

View reviewed changes

tmacam reviewed May 16, 2023

View reviewed changes

DeepanshuA added 11 commits May 18, 2023 09:12

Incorporate Review comments

4886c9e

Signed-off-by: Deepanshu Agarwal <deepanshu.agarwal1984@gmail.com>

Lint, validate

18bc883

Signed-off-by: Deepanshu Agarwal <deepanshu.agarwal1984@gmail.com>

test correction

9d8ec7f

Signed-off-by: Deepanshu Agarwal <deepanshu.agarwal1984@gmail.com>

Fake class method correction

49f960a

Signed-off-by: Deepanshu Agarwal <deepanshu.agarwal1984@gmail.com>

Check expected std output in validate example

c64a091

Signed-off-by: Deepanshu Agarwal <deepanshu.agarwal1984@gmail.com>

Remove extra port check

f3207fc

Signed-off-by: Deepanshu Agarwal <deepanshu.agarwal1984@gmail.com>

Merge branch 'master' of github.com:dapr/python-sdk into workflow_aut…

2bfc44a

…horing

Temporary - Verify Workflow Example first

82b33e5

Signed-off-by: Deepanshu Agarwal <deepanshu.agarwal1984@gmail.com>

Requirements

de33a9b

Signed-off-by: Deepanshu Agarwal <deepanshu.agarwal1984@gmail.com>

Remove line

363a0df

Signed-off-by: Deepanshu Agarwal <deepanshu.agarwal1984@gmail.com>

Add back removed validate examples

ba2087a

Signed-off-by: Deepanshu Agarwal <deepanshu.agarwal1984@gmail.com>

berndverst reviewed May 22, 2023

View reviewed changes

examples/demo_workflow/demo_workflow/requirements.txt Outdated Show resolved Hide resolved

berndverst and others added 2 commits May 22, 2023 13:37

Update examples/demo_workflow/demo_workflow/requirements.txt

a3fb750

Signed-off-by: Bernd Verst <github@bernd.dev>

Change running order of wf

135d3d7

Signed-off-by: Deepanshu Agarwal <deepanshu.agarwal1984@gmail.com>

Commit to re-run example

a9be483

Signed-off-by: Deepanshu Agarwal <deepanshu.agarwal1984@gmail.com>

cgillum mentioned this pull request May 23, 2023

Fix regression caused by durabletask-go v0.2.2 update dapr/dapr#6390

Merged

7 tasks

berndverst approved these changes May 23, 2023

View reviewed changes

berndverst merged commit e27dac9 into dapr:master May 23, 2023

berndverst added this to the v1.10 milestone Jun 5, 2023

Workflow Authoring #559

Workflow Authoring #559

Uh oh!

Conversation

DeepanshuA commented Apr 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Issue reference

Checklist

Uh oh!

codecov bot commented Apr 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

DeepanshuA commented Apr 25, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

berndverst left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

berndverst commented May 10, 2023

Uh oh!

Uh oh!

tmacam May 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DeepanshuA commented May 16, 2023

Uh oh!

cgillum left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DeepanshuA commented Apr 25, 2023 •

edited

Loading

codecov bot commented Apr 25, 2023 •

edited

Loading

tmacam May 16, 2023 •

edited

Loading

berndverst commented May 22, 2023 •

edited

Loading