Fix workflow subOrchestration creation #7385
Conversation
Signed-off-by: Shivam Kumar <shivamkm07@gmail.com>
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #7385 +/- ##
==========================================
- Coverage 62.21% 62.17% -0.05%
==========================================
Files 239 239
Lines 21942 21950 +8
==========================================
- Hits 13652 13647 -5
- Misses 7158 7174 +16
+ Partials 1132 1129 -3 ☔ View full report in Codecov by Sentry. |
| @@ -598,12 +598,21 @@ func (wf *workflowActor) runWorkflow(ctx context.Context, actorID string, remind | |||
| if errMarshal != nil { | |||
| return errMarshal | |||
| } | |||
|
|
|||
| requestBytes := eventData | |||
There was a problem hiding this comment.
Is this not backwards-incompatible? If a sidecar that is running 1.12 receives this message, it may not know how to parse this?
There was a problem hiding this comment.
I think not. If a sidecar is running 1.12, here we will be sending only startEvent as rawData: https://github.com/dapr/dapr/blob/release-1.12/pkg/runtime/wfengine/workflow.go#L485
and then we would be parsing that only in CreateWorkflowInstanceMethod:
dapr/pkg/runtime/wfengine/workflow.go
Line 170 in 986d38b
The issue here is PR #7308 added the struct here:
but missed to add here and so the inconsistency causing the issue.There was a problem hiding this comment.
So #7308 is not backwards-compatible?
There was a problem hiding this comment.
I meant #7308 is backwards compatible. It's just that it introduced a bug and the current PR fixes it. But it should worknwell even if the sidecar is from 1.12.
|
If this was a regression, we should consider adding a unit test in |
Signed-off-by: Shivam Kumar <shivamkm07@gmail.com>
Added |
|
/test-version-skew |
Dapr Version Skew e2e test (control-plane-master - 1.12.3)Commit ref: 24613a4 ✅ Version Skew tests passed |
Dapr Version Skew integration test (dapr-sidecar-master - 1.12.3)Commit ref: 24613a4 ✅ Version Skew tests passed |
Dapr Version Skew e2e test (dapr-sidecar-master - 1.12.3)Commit ref: 24613a4 ❌ Version Skew tests failedPlease check the logs for details on the error. |
Dapr Version Skew integration test (control-plane-master - 1.12.3)Commit ref: 24613a4 ✅ Version Skew tests passed |
|
/test-sdk-all |
|
/ok-to-test |
Dapr SDK Go testCommit ref: 24613a4 ✅ Go SDK tests passed |
Dapr SDK Java testCommit ref: 24613a4 ❌ Java SDK tests failedPlease check the logs for details on the error. |
Dapr SDK Python testCommit ref: 24613a4 ✅ Python SDK tests passed |
Dapr E2E testCommit ref: 24613a4 ✅ Infrastructure deployed
✅ Build succeeded for linux/amd64
✅ Build succeeded for windows/amd64
❌ Tests failed on windows/amd64Please check the logs for details on the error. ❌ Tests failed on linux/amd64Please check the logs for details on the error. |
Dapr SDK JS testCommit ref: 24613a4 ✅ JS SDK tests passed |
|
/ok-to-test |
Dapr E2E testCommit ref: 187f105 ✅ Build succeeded for linux/amd64
✅ Infrastructure deployed
✅ Build succeeded for windows/amd64
❌ Tests failed on windows/amd64Please check the logs for details on the error. ❌ Tests failed on linux/amd64Please check the logs for details on the error. |
* fix workflow subOrchestration Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * Adding UT for child workflow creation Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> --------- Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> Co-authored-by: Mukundan Sundararajan <65565396+mukundansundar@users.noreply.github.com> Signed-off-by: Cassandra Coyle <cassie@diagrid.io>
Description
It fixes workflow sub-orchestration creation by passing correctly the
CreateWorkflowInstanceRequestat the time of Sub-orchestration creation. The struct was added as part of #7308.The PR also adds integration test for the same.
Issue reference
Please reference the issue this PR will close: #7384
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: