Skip to content

[Workflow] Improve management API usability#1087

Merged
halspang merged 4 commits into
dapr:masterfrom
cgillum:workflow-management
May 12, 2023
Merged

[Workflow] Improve management API usability#1087
halspang merged 4 commits into
dapr:masterfrom
cgillum:workflow-management

Conversation

@cgillum
Copy link
Copy Markdown
Contributor

@cgillum cgillum commented May 10, 2023

Description

This PR contains breaking changes to the v1.10 workflow management client APIs. The point of these changes is to improve the usability of these core APIs. This includes adding a few additional convenience types and methods.

This PR also updates the console app sample, replacing the DaprWorkflowClient with DaprClient, which was always the intent.

Issue reference

Resolves #1027.

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

FYI @RyanLettieri

Signed-off-by: Chris Gillum <cgillum@microsoft.com>
@cgillum cgillum requested review from a team as code owners May 10, 2023 00:59
@codecov
Copy link
Copy Markdown

codecov Bot commented May 10, 2023

Codecov Report

Merging #1087 (5c55396) into master (610632a) will decrease coverage by 0.57%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #1087      +/-   ##
==========================================
- Coverage   68.07%   67.51%   -0.57%     
==========================================
  Files         169      170       +1     
  Lines        5604     5652      +48     
  Branches      592      600       +8     
==========================================
+ Hits         3815     3816       +1     
- Misses       1649     1696      +47     
  Partials      140      140              
Flag Coverage Δ
net6 67.42% <0.00%> (-0.58%) ⬇️
net7 67.42% <0.00%> (-0.58%) ⬇️
netcoreapp3.1 67.48% <0.00%> (-0.57%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/Dapr.Client/DaprClient.cs 73.52% <0.00%> (-19.07%) ⬇️
src/Dapr.Client/DaprClientGrpc.cs 74.17% <0.00%> (-0.95%) ⬇️
src/Dapr.Client/GetWorkflowResponse.cs 0.00% <0.00%> (ø)
src/Dapr.Client/WorkflowFailureDetails.cs 0.00% <0.00%> (ø)

Comment thread examples/Workflow/WorkflowConsoleApp/Program.cs Outdated
Comment thread src/Dapr.Client/GetWorkflowResponse.cs Outdated
@RyanLettieri
Copy link
Copy Markdown
Contributor

While you didn't touch the file, we should update the asserts inside WorkflowTest.cs to use the enumeration: "WorkflowRuntimeStatus" for the status of the runtime rather than hardcoded values such as "RUNNING", "SUSPENDED", etc.

@cgillum
Copy link
Copy Markdown
Contributor Author

cgillum commented May 10, 2023

While you didn't touch the file, we should update the asserts inside WorkflowTest.cs to use the enumeration: "WorkflowRuntimeStatus" for the status of the runtime rather than hardcoded values such as "RUNNING", "SUSPENDED", etc.

Oh, interesting. I've never used FluentAssertions before, but it seems like a weakness in its design. Ideally the compiler would have caught this. I'll go ahead and fix that, along with your other suggestions.

Copy link
Copy Markdown
Contributor

@halspang halspang left a comment

Choose a reason for hiding this comment

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

Mostly nitpicks, overall seems fine.

Comment thread src/Dapr.Client/DaprClient.cs Outdated
Comment thread src/Dapr.Client/DaprClient.cs Outdated
Comment thread src/Dapr.Client/DaprClient.cs
Comment thread src/Dapr.Client/GetWorkflowResponse.cs Outdated
Comment thread src/Dapr.Workflow/DaprWorkflowClient.cs Outdated
Comment thread test/Dapr.E2E.Test/Workflows/WorkflowTest.cs Outdated
Signed-off-by: Chris Gillum <cgillum@microsoft.com>
@halspang halspang merged commit 8152c74 into dapr:master May 12, 2023
@cgillum cgillum deleted the workflow-management branch May 12, 2023 23:09
onionhammer pushed a commit to onionhammer/dotnet-sdk that referenced this pull request May 30, 2023
* [Workflow] Improve management API usability

Signed-off-by: Chris Gillum <cgillum@microsoft.com>

* PR feedback and update E2E test

Signed-off-by: Chris Gillum <cgillum@microsoft.com>

* PR feedback

Signed-off-by: Chris Gillum <cgillum@microsoft.com>

---------

Signed-off-by: Chris Gillum <cgillum@microsoft.com>
Signed-off-by: Erik O'Leary <erik.m.oleary@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Workflow] Improvements to Dapr Client APIs for Workflow

3 participants