cron: fix day-of-week off-by-one in crontab schedules#138
Merged
Conversation
APScheduler's numeric day_of_week is 0=Mon..6=Sun, while Unix crontab is 0=Sun..6=Sat (7 also means Sun). CronTrigger.from_crontab passes the number straight through without remapping, so every numeric day-of-week schedule fired one weekday late: "0 13 * * 1" (Monday) actually ran on Tuesday, and "0 3 * * 0" (Sunday) ran on Monday. Add _crontab_to_trigger, a from_crontab replacement that translates the day-of-week field to APScheduler's day-name aliases (preserving *, ranges like 1-5, lists like 1,4, and step suffixes) and leaves the other four fields and the timezone handling unchanged. Route the three trigger-construction sites (job scheduling, source scheduling, and the overdue check) through it. Tests cover the day-of-week mapping, parity with from_crontab for non-DOW schedules, the interval-string fallback, and a weekly _is_overdue case that is due after exactly one week. After a restart, every numeric day-of-week cron shifts to the day its schedule already specifies; that is the intended correction.
alex-fedotyev
force-pushed
the
alex/cron-dow-fix
branch
from
c3258f6 to
ac944fb
Compare
pufit
approved these changes
Jun 23, 2026
pufit
added a commit
to fallintoplace/nerve
that referenced
this pull request
Jun 23, 2026
Resolve conflicts with ClickHouse#138 (_crontab_to_trigger): thread the configured timezone through the helper at all call sites instead of reverting to host-tz CronTrigger.from_crontab. Also set config.timezone in the wakeup-sweep test fixture so CronService.__init__ accepts it.
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
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.
Summary
APScheduler's numeric
day_of_weekis0=Mon..6=Sun, but Unix crontab uses0=Sun..6=Sat(and7also means Sunday).CronTrigger.from_crontab()passes the number straight through without remapping, so every cron whose schedule uses a numeric day-of-week fires one weekday late. The shipped weeklyskill-reviserjob (0 3 * * 0, intended Sunday) actually fires Monday, and0 13 * * 1(Monday) fires Tuesday.This adds
_crontab_to_trigger(), a drop-in replacement forfrom_crontab()that translates the day-of-week field to APScheduler's day-name aliases (mon,tue, and so on). It preserves*, ranges (1-5), lists (1,4), and step suffixes (*/2), and leaves the other four fields and the timezone behaviour exactly as before. The three places that build a trigger from a crontab string now route through it:_make_trigger)_is_overdue)Only the day-of-week interpretation changes. A schedule without a numeric day-of-week field produces the same trigger as before; there is a test asserting parity with
from_crontab.Rollout note
Schedules are read at startup, so this takes effect on the next daemon restart. After the restart, every numeric day-of-week cron shifts to the weekday its schedule already specifies. That is the intended correction, not a change to the schedule strings.
A follow-up will harden startup catch-up so a weekly job that has never recorded a successful run can still be backfilled after a missed fire.
Test plan
pytest tests/test_cron.py: 82 passed locally.TestCrontabToTriggercovers numeric Monday/Sunday/7, range1-5(Mon to Fri), list1,4(Mon and Thu),*(every day), day-name passthrough, parity withfrom_crontabfor non-DOW schedules, and the interval-string fallback still raisingValueError._is_overduecase proves a weekly Monday job is overdue after exactly one week, not 6 or 8 days.from_crontab("0 13 * * 1")returns a Tuesday while_crontab_to_trigger("0 13 * * 1")returns a Monday.