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

[Workflow] Fix issue with ignored external event payload #1119

Merged
merged 3 commits into from
Jul 6, 2023

Conversation

cgillum
Copy link
Contributor

@cgillum cgillum commented Jul 3, 2023

Description

In dapr/dapr#6614, a user observed that the RaiseEventAsync method was always resulting in a null payload in the workflow code. This PR fixes the issue, which is isolated to the Dapr Workflow SDK.

I also made a couple other small changes as part of this PR:

  • Bumped the Durable Task SDK dependency to the latest patch release.
  • Changed the default getInputsAndOutputs to true for the workflow status query APIs (I decided to do this to improve usability after some frustration while setting up the local repro for the raise event issue)

I also updated the Workflow SDK nuget version to 0.3.0.

Issue reference

Closes dapr/dapr#6614

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: Chris Gillum <cgillum@microsoft.com>
@cgillum cgillum requested review from a team as code owners July 3, 2023 20:51
Signed-off-by: Chris Gillum <cgillum@microsoft.com>
@codecov
Copy link

codecov bot commented Jul 3, 2023

Codecov Report

Merging #1119 (e8074a4) into master (8e9db70) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1119   +/-   ##
=======================================
  Coverage   67.22%   67.22%           
=======================================
  Files         170      170           
  Lines        5687     5687           
  Branches      605      605           
=======================================
  Hits         3823     3823           
  Misses       1723     1723           
  Partials      141      141           
Flag Coverage Δ
net6 67.15% <ø> (ø)
net7 67.15% <ø> (ø)
netcoreapp3.1 67.18% <ø> (ø)

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

@halspang halspang added this to the v1.12 milestone Jul 5, 2023
@halspang halspang added kind/bug Something isn't working area/workflow labels Jul 5, 2023
@halspang
Copy link
Contributor

halspang commented Jul 5, 2023

@cgillum - You have test failures that I think are related to legacy steps in the itests workflow. Can you remove these steps and repush to see if that fixes things?

https://github.com/dapr/dotnet-sdk/blob/master/.github/workflows/itests.yml#L108-L116

Signed-off-by: Chris Gillum <cgillum@microsoft.com>
@halspang halspang merged commit c99475b into dapr:master Jul 6, 2023
12 checks passed
MonkeyTennis pushed a commit to MonkeyTennis/dotnet-sdk that referenced this pull request Jul 14, 2023
* [Workflow] Fix issue with ignored external event payload

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

* Pushing missing commits

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

* Remove unnecessary steps from itests.yml

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

---------

Signed-off-by: Chris Gillum <cgillum@microsoft.com>
Signed-off-by: Bradley Cotier <bcotier@microsoft.com>
MonkeyTennis pushed a commit to MonkeyTennis/dotnet-sdk that referenced this pull request Jul 14, 2023
* [Workflow] Fix issue with ignored external event payload

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

* Pushing missing commits

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

* Remove unnecessary steps from itests.yml

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

---------

Signed-off-by: Chris Gillum <cgillum@microsoft.com>
Signed-off-by: Bradley Cotier <bcotier@microsoft.com>
MonkeyTennis pushed a commit to MonkeyTennis/dotnet-sdk that referenced this pull request Jul 14, 2023
* [Workflow] Fix issue with ignored external event payload

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

* Pushing missing commits

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

* Remove unnecessary steps from itests.yml

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

---------

Signed-off-by: Chris Gillum <cgillum@microsoft.com>
Signed-off-by: Bradley Cotier <bcotier@microsoft.com>
artursouza pushed a commit to artursouza/dotnet-sdk that referenced this pull request Jul 18, 2023
* [Workflow] Fix issue with ignored external event payload

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

* Pushing missing commits

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

* Remove unnecessary steps from itests.yml

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

---------

Signed-off-by: Chris Gillum <cgillum@microsoft.com>
Signed-off-by: Artur Souza <asouza.pro@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/workflow kind/bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Workflows] Raise event payload is always null in my sample app
2 participants