Skip to content
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

feat: implement workflow management #476

Closed
wants to merge 7 commits into from

Conversation

mikeee
Copy link
Member

@mikeee mikeee commented Nov 13, 2023

[WIP]
Implements Workflow management (Alpha + Beta endpoints).

E2E testing is not yet implemented as Authoring is still outstanding.

This does not yet fully close this issue: #379 as authoring is not included in this PR.
Contributes towards the completion of dapr/dapr#7156

  • Testing implemented
  • Checks/Tests passing
  • Documentation updated/improved

Signed-off-by: mikeee <hey@mike.ee>
@mikeee mikeee requested a review from a team as a code owner November 13, 2023 14:15
Copy link

codecov bot commented Nov 13, 2023

Codecov Report

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

Comparison is base (ae8becf) 68.96% compared to head (d9b7adc) 72.22%.
Report is 2 commits behind head on main.

Files Patch % Lines
client/workflow.go 96.02% 8 Missing and 4 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #476      +/-   ##
==========================================
+ Coverage   68.96%   72.22%   +3.25%     
==========================================
  Files          34       36       +2     
  Lines        2707     3143     +436     
==========================================
+ Hits         1867     2270     +403     
- Misses        732      754      +22     
- Partials      108      119      +11     

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

@mikeee mikeee marked this pull request as draft November 13, 2023 14:20
@yaron2
Copy link
Member

yaron2 commented Nov 14, 2023

cc @cgillum

client/workflow.go Outdated Show resolved Hide resolved
Signed-off-by: mikeee <hey@mike.ee>
Signed-off-by: mikeee <hey@mike.ee>
Signed-off-by: mikeee <hey@mike.ee>
@yaron2
Copy link
Member

yaron2 commented Nov 16, 2023

Thanks for picking up the review here @RyanLettieri

Choose a reason for hiding this comment

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

One test case I would like to see is passing in an input to the start workflow calls. In other SDKs we've run into an issue where certain input types would lead to an error since they couldn't be parsed correctly. For example, in .NET we needed to serialize the data and convert it into a bytestring. It may not be an issue in GO, but having the test coverage can't hurt.

client/workflow.go Outdated Show resolved Hide resolved
@mikeee mikeee force-pushed the mikeee-feat-workflow-management branch from b834f30 to 55a388e Compare December 5, 2023 18:15
Signed-off-by: mikeee <hey@mike.ee>
@mikeee mikeee force-pushed the mikeee-feat-workflow-management branch from 55a388e to 99c2747 Compare December 5, 2023 18:21
Signed-off-by: mikeee <hey@mike.ee>
Signed-off-by: mikeee <hey@mike.ee>
@mikeee mikeee marked this pull request as ready for review January 2, 2024 17:32
@mikeee
Copy link
Member Author

mikeee commented Jan 4, 2024

Merging these commits into #487

@mikeee mikeee closed this Jan 4, 2024
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.

None yet

3 participants