Skip to content

feat: workflow authoring and management support #487

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 121 commits into from
Feb 9, 2024
Merged

feat: workflow authoring and management support #487

merged 121 commits into from
Feb 9, 2024

Conversation

@mikeee
Copy link
Member

@mikeee mikeee commented Dec 17, 2023 •
edited
Loading

Implements Workflow authoring using durabletask-go and also an interface to the dapr grpc management endpoints.

Contributes towards the completion of dapr/dapr#7156 and closes #379

This PR also exposes the GRPC connection from the dapr client.

Requires documentation in dapr/docs#3895

  • Testing implemented (E2E)
  • Checks/Tests passing
  • Documentation updated/improved

@mikeee mikeee force-pushed the mikeee-feat-workflow-authoring branch from facf3ee to 3ae03a4 Compare December 17, 2023 15:48
@codecov
Copy link

codecov bot commented Dec 17, 2023 •
edited
Loading

Codecov Report

Attention: 290 lines in your changes are missing coverage. Please review.

Comparison is base (cefbadb) 70.11% compared to head (6f5aa89) 59.37%.
Report is 1 commits behind head on main.

Files Patch % Lines
examples/workflow/main.go 0.00% 182 Missing ⚠️
workflow/client.go 50.61% 38 Missing and 2 partials ⚠️
workflow/worker.go 55.38% 25 Missing and 4 partials ⚠️
workflow/workflow.go 54.16% 21 Missing and 1 partial ⚠️
workflow/context.go 50.00% 17 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff             @@
##             main     #487       +/-   ##
===========================================
- Coverage   70.11%   59.37%   -10.74%     
===========================================
  Files          35       54       +19     
  Lines        2884     3483      +599     
===========================================
+ Hits         2022     2068       +46     
- Misses        748     1293      +545     
- Partials      114      122        +8

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mikeee mikeee changed the title feat: workflow authoring support feat: workflow authoring support [very wip] Dec 19, 2023
@mikeee mikeee force-pushed the mikeee-feat-workflow-authoring branch from d9f8f1c to 74cbdbc Compare January 2, 2024 17:24
@mikeee mikeee mentioned this pull request Jan 4, 2024
Closed
3 tasks
@mikeee mikeee changed the title feat: workflow authoring support [very wip] feat: workflow authoring and management support Jan 4, 2024
@mikeee mikeee force-pushed the mikeee-feat-workflow-authoring branch 18 times, most recently from f166caf to 328c94a Compare January 9, 2024 15:45
@mikeee mikeee marked this pull request as ready for review January 11, 2024 00:25
@mikeee mikeee requested a review from a team as a code owner January 11, 2024 00:25
mukundansundar
mukundansundar reviewed Jan 12, 2024
View reviewed changes
Copy link

@mukundansundar mukundansundar left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Great start with adding in workflow support with Dapr management APIs. Just left a few comments and suggestions.

  • consider adding authoring SDK
  • should generics support be added instead of any interface{} for input/output as if there are plans to add it later on that might be a breaking change? If breaking changes are ok, then this can be a future feature.

cc @yaron2 wdyt?

@mikeee mikeee force-pushed the mikeee-feat-workflow-authoring branch 3 times, most recently from f5fa833 to 947414d Compare January 14, 2024 14:39
@mikeee mikeee requested a review from cgillum January 30, 2024 12:36
@yaron2
Copy link
Member

yaron2 commented Jan 31, 2024

Looks like we can merge this. @cgillum

@mikeee mikeee mentioned this pull request Jan 31, 2024
Merged
4 tasks
cgillum
cgillum reviewed Feb 7, 2024
View reviewed changes
Copy link
Contributor

@cgillum cgillum left a comment

Choose a reason for hiding this comment

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

Working my way through this. Adding a few comments now before I finish reviewing.

cgillum
cgillum reviewed Feb 7, 2024
View reviewed changes
Copy link
Contributor

@cgillum cgillum left a comment

Choose a reason for hiding this comment

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

Just a couple super-minor things. The most important is probably changing the wording in the documentation comments regarding .Await(...) as I'm a bit worried that users might misunderstand how tasks can be used.

mikeee and others added 3 commits February 7, 2024 10:53
@mikeee @cgillum
implements correction
99aa6ed 
Co-authored-by: Chris Gillum <[email protected]>
Signed-off-by: mikeee <[email protected]>
@mikeee
change typecast assertion
bac187e 
Signed-off-by: mikeee <[email protected]>
@mikeee
improve clarity of notes
2b0d4ca 
Signed-off-by: mikeee <[email protected]>
@mikeee mikeee requested a review from cgillum February 7, 2024 12:47
cgillum
cgillum approved these changes Feb 7, 2024
View reviewed changes
Copy link
Contributor

@cgillum cgillum left a comment

Choose a reason for hiding this comment

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

I'm signing off on this but with a couple final recommendations.

I appreciate the hard work that went into this. :)

mikeee added 4 commits February 7, 2024 20:05
@mikeee
Merge branch 'main' into mikeee-feat-workflow-authoring
9e9fcc9 
Signed-off-by: mikeee <[email protected]>
@mikeee
fix mod issues from rebasing interactively on github
20886a8 
Signed-off-by: mikeee <[email protected]>
@mikeee
implement suggestions from review
901dff0 
- task invoke documentation
- refactor type assertion for startworkflowbeta1

Signed-off-by: mikeee <[email protected]>
@mikeee
remoove unused definition
69bfe66 
Signed-off-by: mikeee <[email protected]>
@mikeee
Copy link
Member Author

mikeee commented Feb 7, 2024

@yaron2 for your eyes

@yaron2
Copy link
Member

yaron2 commented Feb 8, 2024

@mikeee please resolve the conflicts and we can merge

@mikeee
Copy link
Member Author

mikeee commented Feb 8, 2024

I had a feeling this would happen 😂

@mikeee
Copy link
Member Author

mikeee commented Feb 8, 2024

@yaron2 good to go, not sure why codecov is unhappy but 🤷

@yaron2 yaron2 merged commit ac26e62 into dapr:main Feb 9, 2024
@mikeee mikeee deleted the mikeee-feat-workflow-authoring branch February 9, 2024 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yaron2 yaron2 yaron2 approved these changes

@mukundansundar mukundansundar Awaiting requested review from mukundansundar

+2 more reviewers

@cgillum cgillum cgillum approved these changes

@famarting famarting famarting approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Workflow implementation in SDK

5 participants

@mikeee @yaron2 @famarting @cgillum @mukundansundar