workflows, remove deprecated functions #749
Conversation
Signed-off-by: Fabian Martinez <[email protected]>
| @@ -1 +1 @@ | |||
| dapr-ext-workflow-dev>=0.0.1rc1.dev | |||
| dapr-ext-workflow-dev>=0.4.1rc1.dev | |||
There was a problem hiding this comment.
Please do not modify this in the future. We want to run examples against older versions for backwards compatibility as well. It's fine here given the SDK is not 1.0 yet.
examples/demo_workflow/app.py
Outdated
| @@ -106,7 +107,7 @@ def act_for_child_wf(ctx: WorkflowActivityContext, inp): | |||
|
|
|||
|
|
|||
| def main(): | |||
| with DaprClient() as d: | |||
| with DaprWorkflowClient() as wfc: | |||
There was a problem hiding this comment.
This is changing what this example (which also runs as an integration test) does!
The point was that you can use the regular Dapr SDK to start a workflow as well - you do not have to use the workflow SDK for that. We will want to retain this.
There was a problem hiding this comment.
this is equivalent to dapr/dotnet-sdk#1344 and issues like this have been created in all other SDKs, the workflow functions in the DaprClient are now deprecated and we shouldn't advice users to use them. Thats why changing the examples.
There was a problem hiding this comment.
Since this example is also an integration test, as Bernd said, I'd prefer adding a deprecation notice and keeping it until we completely remove support for workflows in the DaprClient.
We have other examples/tests for thedapr-ext-workflow client in the https://github.com/dapr/python-sdk/tree/main/examples/workflow directory.
So I'd suggest checking this file for overlaps with whatever is already in there and moving it to that dir.
If we're doing this deprecation work in all clients, we should also go through the docs and remove all references to DaprClient in the context of workflows and only use dapr-ext-workflow. As a matter of fact, someone just opened an issue for that a few days ago: dapr/docs#4410
FYI @hhunter-ms
There was a problem hiding this comment.
the point of changing this is that we no longer want to encourage users to use the DaprClient to interact with Workflows.
examples usually are the first point of contact for users, so intentionally leaving a deprecated example just because it acts as a test is a mistake in my opinion.
There was a problem hiding this comment.
but since we already have examples of the correct SDK to use here https://github.com/dapr/python-sdk/tree/main/examples/workflow
I'll revert the changes made to this file
There was a problem hiding this comment.
@famarting @elena-kolevska will the Python workflow quickstart also be updated to dapr-ext-workflow instead of DaprClient?
There was a problem hiding this comment.
wow, I didnt realize of that, well, all quickstarts in that repo using the equivalent to the DaprClient will also need to be updated
Signed-off-by: Fabian Martinez <[email protected]>
|
ping |
| @@ -1408,6 +1408,7 @@ async def start_workflow( | |||
| send_raw_bytes: bool = False, | |||
| ) -> StartWorkflowResponse: | |||
| """Starts a workflow. | |||
| Deprecated: use dapr-ext-workflow instead | |||
There was a problem hiding this comment.
In addition to these deprecation notice in the docstring, can you please emit a warning message (please do this the same way we do elsewhere in the SDK).
Nit please add the word SDK or package -- Use the dapr-ext-workflow package.
There was a problem hiding this comment.
Right now we already have:
'The Workflow API is a Beta version and is subject to change.',
So maybe update this to say: This Workflow API (Beta) method is deprecated and will be removed in a future version. Use the dapr-ext-workflow` package instead.
Signed-off-by: Fabian Martinez <[email protected]>
Signed-off-by: Elena Kolevska <[email protected]>
|
ping |
Signed-off-by: Elena Kolevska <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #749 +/- ##
==========================================
- Coverage 86.63% 86.03% -0.60%
==========================================
Files 84 87 +3
Lines 4473 4783 +310
==========================================
+ Hits 3875 4115 +240
- Misses 598 668 +70 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
Description
start of #738
adds comments indicating the workflow functions that are deprecated
updates the existing example app to use the recommended functions
pending updating the contents of daprdocs
pending support for the purge API to finish this work
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 will close: #[issue number]
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: