Add a "scheduled_execution_timestamp" field to schedule tick data #26822
Conversation
> Insert changelog entry or delete this section.
gibsondan
changed the title
gibsondan
force-pushed
the
consecutivefailurecount2
branch
from
0c2a6ef to
ae401a6
Compare
|
Deploy preview for dagit-core-storybook ready! ✅ Preview Built with commit 55088ae. |
Summary: This paves the way to make schedule retry behavior more like sensor retry behavior. Right now if a schedule fails, it retries for the same time using the same tick, and the timestamp of that tick is synonymous with the scheduled time in the cron schedule. This differs from sensor retries. This divergence in behavior is confusing and leads to issues like schedule failure alerts leading nowhere, since the schedule tick retried and succeeded. By adding this field, we can eventually change this behavior so that a retried schedule tick keeps the scheduled_execution_timestamp field, but updates the timestamp field and creates a new tick on the timeline. > Insert changelog entry or delete this section.
gibsondan
force-pushed
the
consecutivefailurecount2
branch
from
ae401a6 to
55088ae
Compare
| @@ -551,6 +563,7 @@ def automation_condition_evaluation_id(self) -> int: | |||
| "instigator_name": "job_name", | |||
| "instigator_type": "job_type", | |||
| }, | |||
| skip_when_none_fields={"scheduled_execution_time"}, | |||
There was a problem hiding this comment.
this is optional but i figured why bloat the serdes object for every sensor tick
|
I want to sanity check some assumptions... can there be an unrelated tick for a different scheduled execution time in between a failed schedule tick and its retry? I'm asking because of this tick fetch, which will always fetch by |
|
no, for a given schedule we only ever move forwards in.. scheduled execution time. it's not quite ready yet, but the next step here will be #26824 - which starts writing the current time instead of the scheduled execution time, so retried ticks will have the same scheduled execution time but a different timestamp. But we would only ever do the most recent scheduled execution timestamp |
gibsondan
force-pushed
the
consecutivefailurecount
branch
3 times, most recently
from
3934081 to
df6681f
Compare
Summary:
This paves the way to make schedule retry behavior more like sensor retry behavior. Right now if a schedule fails, it retries for the same time using the same tick, and the timestamp of that tick is synonymous with the scheduled time in the cron schedule. This differs from sensor retries. This divergence in behavior is confusing and leads to issues like schedule failure alerts leading nowhere, since the schedule tick retried and succeeded.
By adding this field, we can eventually change this behavior so that a retried schedule tick keeps the scheduled_execution_timestamp field, but updates the timestamp field and creates a new tick on the timeline.
Summary & Motivation
How I Tested These Changes
Changelog