-
Notifications
You must be signed in to change notification settings - Fork 15
Support workflow patching #56
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
Conversation
Signed-off-by: Albert Callarisa <albert@diagrid.io>
backend/orchestration.go
Outdated
|
|
||
| if results.GetPatchMismatch() { | ||
| _ = runtimestate.AddEvent(wi.State, &protos.HistoryEvent{ | ||
| EventId: -1, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unrelated note: we should update Event ID to be optional.
Signed-off-by: Albert Callarisa <albert@diagrid.io>
fb69eac to
fd5db26
Compare
Signed-off-by: Albert Callarisa <albert@diagrid.io>
Signed-off-by: Albert Callarisa <albert@diagrid.io>
Signed-off-by: Albert Callarisa <albert@diagrid.io>
Signed-off-by: Albert Callarisa <albert@diagrid.io>
Signed-off-by: Albert Callarisa <albert@diagrid.io>
cicoyle
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
few things from me
|
Can you resolve the conflicts? |
Signed-off-by: Albert Callarisa <albert@diagrid.io>
Signed-off-by: Albert Callarisa <albert@diagrid.io>
Signed-off-by: Albert Callarisa <albert@diagrid.io>
3b7cc51 to
ffdb640
Compare
Signed-off-by: Albert Callarisa <albert@diagrid.io>
ffdb640 to
0a01e95
Compare
Signed-off-by: Albert Callarisa <albert@diagrid.io>
backend/executor.go
Outdated
| } | ||
|
|
||
| func (g *grpcExecutor) requireNotStalled(ctx context.Context, id string) error { | ||
| metadata, err := g.backend.GetOrchestrationMetadata(ctx, api.InstanceID(id)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks to be very expensive to do on every call (?)
Does this need to be done in the executor, and not in dapr actor?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, I moved to the actors in the dapr/dapr PR, this particular commit: dapr/dapr@fc07ce9
Signed-off-by: Albert Callarisa <albert@diagrid.io>
Signed-off-by: Albert Callarisa <albert@diagrid.io>
Signed-off-by: Albert Callarisa <albert@diagrid.io>
cicoyle
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thx 🎉
JoshVanL
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just the one thing
Co-authored-by: Josh van Leeuwen <me@joshvanl.dev> Signed-off-by: Albert Callarisa <albert@acroca.com>
Implements the patching as described in the versioning proposal.
Quick summary of the changes added in this PR:
ctx.IsPatched(patchName) boolto ongoing workflowsPendingVersionif the orchestrator find a patch mismatchPendingVersionevent when a patch mismatch is foundstate.IsPendingVersionacordingly