Add Cron dependency for cron-style task scheduling #311
Conversation
Introduces a new Cron dependency that extends Perpetual to support wall-clock scheduled tasks using cron expressions. Unlike Perpetual which uses relative intervals, Cron schedules tasks at exact times (e.g., "0 9 * * 1" for Mondays at 9 AM). Key changes: - Add Cron class with croniter integration for expression parsing - Support standard 5-field cron syntax and Vixie keywords (@daily, etc.) - Automatic scheduling at worker startup (automatic=True by default) - Fix Perpetual.__aenter__ to preserve the automatic flag Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #311 +/- ##
==========================================
+ Coverage 98.63% 98.65% +0.01%
==========================================
Files 100 102 +2
Lines 10066 10149 +83
Branches 491 491
==========================================
+ Hits 9929 10012 +83
Misses 121 121
Partials 16 16
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
chrisguidry
left a comment
chrisguidry
left a comment
There was a problem hiding this comment.
Everything looks good except the change to Worker, that's what my recent refactorings should have eliminated. Let's do whatever we need to eliminate the need to change Worker, then I'm stoked!
| async def __aenter__(self) -> Perpetual: | ||
| execution = self.execution.get() | ||
| perpetual = Perpetual(every=self.every) | ||
| perpetual = Perpetual(every=self.every, automatic=self.automatic) |
src/docket/worker.py
Outdated
| when = ( | ||
| perpetual.get_next() | ||
| if isinstance(perpetual, Cron) | ||
| else None | ||
| ) |
There was a problem hiding this comment.
Ah okay so here's something we'd want to avoid with the new factoring, needing to look at a specific dependency subclass here in Worker. I don't think you even need this, though, because in on_complete, you're calling self.at(self.get_next()), which will set _next_when and then you call the super().on_complete(...) so you're golden, shouldn't need this at all from what I see. Let me know if that's not true and we'll fix the worker to do that.
There was a problem hiding this comment.
Hi @chrisguidry. Thanks for the quick review!!
I agree, I would like to implement this without having to change worker.py. Perhaps I'm missing something 🙂
When a Cron task with automatic=True (the default) starts:
- Worker calls
_schedule_all_automatic_perpetual_tasks()at startup - This calls
docket.add(task_function, key=key)()with nowhenparameter - No
when= schedule for now - Task runs immediately
on_completethen correctly schedules the next run at_croniter.get_next()
Say you have Cron("0 9 * * 1") (Mondays at 9 AM) and start your worker on a Wednesday, the task will:
- Run immediately on Wednesday (unexpected)
- Then correctly run every Monday at 9 AM going forward
We could add a method like get_initial_when on the Perpetual class that returns None and get_next() for Cron which can be used for initially scheduling all tasks. That way we avoid the isinstance check in worker.py.
Let me know what you think!
There was a problem hiding this comment.
Ohhh gotcha gotcha, so this is about the automatic scheduling of crons. I agree that Crons should default to automatic=True, that makes perfect sense.
Yes I think what you're describing is exactly what we need: a way for Perpetuals to specify when they should initially start (defaulting to right now). I think that would be a straightforward change right here, and it would open the path for other types of Perpetuals that hit this same design constraint. Nice!
| ] | ||
| dependencies = [ | ||
| "cloudpickle>=3.1.1", | ||
| "croniter>=6", |
There was a problem hiding this comment.
Thanks for pointing me to the new pallets repo, it's unfortunate that the pypi page still has that huge warning on it. I'm good with this!
@zzstoatzz I don't know if you realized this, but croniter lives! https://github.com/pallets-eco/croniter They just haven't removed the goofy warning on PyPI, but there's a bug report about it.
Maybe time to go back in Prefect?
|
Oh also don't worry about the documentation build failing, that's a bug with outside forks that I still need to fix. |
chrisguidry
left a comment
chrisguidry
left a comment
There was a problem hiding this comment.
NICE! Thank you for this, I think it's great, will get a release out soon
| return cron | ||
|
|
||
| @property | ||
| def initial_when(self) -> datetime: |
|
Thanks @chrisguidry! I actually thought of an idea to add custom TZ support. Let me put up another PR and lemme know if you think we can sneak it in before the next release |
|
@chrisguidry Here's the follow-up PR 🙂 #312 |
3 participants
Introduces a new
Crondependency that extendsPerpetualto support wall-clock scheduled tasks using cron expressions. UnlikePerpetualwhich uses relative intervals,Cronschedules tasks at exact times (e.g., "0 9 * * 1" for Mondays at 9 AM).Key changes:
Cronclass withcroniterintegration for expression parsingautomatic=Trueby default)Notes:
croniteras a direct dependency andtypes-croniteras adevdependencytest_cronare mocked to avoid long wait times.Closes #288