Support workflow patching #56
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Albert Callarisa <albert@diagrid.io>
JoshVanL
requested changes
Nov 25, 2025
api/orchestration.go
Outdated
| RUNTIME_STATUS_TERMINATED OrchestrationStatus = protos.OrchestrationStatus_ORCHESTRATION_STATUS_TERMINATED | ||
| RUNTIME_STATUS_PENDING OrchestrationStatus = protos.OrchestrationStatus_ORCHESTRATION_STATUS_PENDING | ||
| RUNTIME_STATUS_SUSPENDED OrchestrationStatus = protos.OrchestrationStatus_ORCHESTRATION_STATUS_SUSPENDED | ||
| RUNTIME_STATUS_PENDING_VERSION OrchestrationStatus = protos.OrchestrationStatus_ORCHESTRATION_STATUS_PENDING_VERSION |
There was a problem hiding this comment.
Would it not be better to call this PENDING_PATCH_MISSMATCH ?
backend/runtimestate/runtimestate.go
Outdated
| return protos.OrchestrationStatus_ORCHESTRATION_STATUS_SUSPENDED | ||
| } else if s.CompletedEvent != nil { | ||
| return s.CompletedEvent.GetOrchestrationStatus() | ||
| } else if s.IsPendingVersion { |
backend/orchestration.go
Outdated
|
|
||
| if results.GetPatchMismatch() { | ||
| _ = runtimestate.AddEvent(wi.State, &protos.HistoryEvent{ | ||
| EventId: -1, |
There was a problem hiding this comment.
Unrelated note: we should update Event ID to be optional.
backend/orchestration.go
Outdated
| ExecutionPendingVersion: &protos.ExecutionPendingVersionEvent{}, | ||
| }, | ||
| }) | ||
| w.logger.Warnf("%v: patch mismatch detected; setting version pending for orchestration instance", wi.InstanceID) |
There was a problem hiding this comment.
Is this log necessary? At minimum it should be debug I think.
| if os := e.GetOrchestratorStarted(); os != nil { | ||
| os.Patches = results.Patches | ||
| for _, p := range results.Patches.Patches { | ||
| if err := helpers.StartAndEndNewPatchSpan(ctx, p, e.Timestamp.AsTime()); err != nil { |
There was a problem hiding this comment.
Is there an existing span we would add the patches as attributes to?
client/worker_grpc.go
Outdated
| resp.Actions = results.Actions | ||
| resp.CustomStatus = results.GetCustomStatus() | ||
| resp.Patches = results.GetPatches() | ||
| resp.PatchMismatch = results.GetPatchMismatch() |
There was a problem hiding this comment.
Should we signal what patches were mismatched?
task/orchestrator.go
Outdated
| } | ||
| scheduleTask := a.GetScheduleTask() | ||
| if scheduleTask.GetName() != ts.GetName() { | ||
| if len(ctx.patches) > 0 { |
There was a problem hiding this comment.
I don’t think I understand how this is working 🤔
Patch miss match should be evaluated server side.
task/orchestrator.go
Outdated
| } | ||
|
|
||
| func (ctx *OrchestrationContext) IsPatched(patchName string) bool { | ||
| isPatched := slices.Contains(ctx.patches, patchName) |
JoshVanL
reviewed
Nov 25, 2025
task/orchestrator.go
Outdated
| } | ||
|
|
||
| // We're at the end of the history stream, we can run the patched version and save the decision for next rerun | ||
| ctx.newPatches = append(ctx.newPatches, patchName) |
There was a problem hiding this comment.
Can we use the variable name of patchesTaken and newPatchesTaken?
Signed-off-by: Albert Callarisa <albert@diagrid.io>
acroca
force-pushed
the
workflow-versioning
branch
from
fb69eac to
fd5db26
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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