-
Notifications
You must be signed in to change notification settings - Fork 2
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
Replay cancel scheduled cleared reruns #283
Conversation
Mryashbhardwaj
commented
Oct 18, 2024
•
edited
Loading
edited
- replay should not cancel non replay managed runs
- also cancel cleared runs(one's that were originally scheduled but cleared by replay)
…d runs(one's that were originally scheduled but cleared by replay)
…ncel_scheduled_reruns
@@ -235,15 +234,17 @@ func (r *ReplayService) cancelReplayRuns(ctx context.Context, replayWithRun *sch | |||
return err | |||
} | |||
|
|||
syncedRunStatus, err := r.executor.SyncStatus(ctx, replayWithRun, jobCron) | |||
jobRunsRunsWithDetails, err := r.executor.FetchRunsWithDetails(ctx, replay, jobCron) |
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.
typo RunsRuns
?
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.
fixed
logicalTime := run.GetLogicalTime(jobCron) | ||
runState := scheduler.JobRunStatus{ | ||
ScheduledAt: run.ScheduledAt, | ||
State: run.State, |
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.
Let's verify whether the canceled runs will still have the origin state or the updated ones. I believe we are expecting canceled
status?
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.
yeah you are right this should be scheduler.StateCanceled
* fix: replay do not cancel non replay managed runs, also cancel cleared runs(one's that were originally scheduled but cleared by replay) * fix: typos and add logs