Initial draft of composable timetables#28757
Initial draft of composable timetables#28757collinmcnulty wants to merge 2 commits intoapache:mainfrom
Conversation
collinmcnulty
force-pushed
the
composable-timetables
branch
from
31ce9c8 to
1b11250
Compare
| def compose_timetable( | ||
| run_strategy_cls: type[RunStrategy], | ||
| interval_strategy_cls: type[IntervalStrategy], | ||
| manual_strategy_cls: type[ManualStrategy], | ||
| ): |
There was a problem hiding this comment.
Ikind of feel this method is a bit redundant and can simply be implemented with a class…?
|
The problem is that, for serialization, each timetable has to be a new class, not a new instance of ComposableTimetable. I could make a factory class but that seems even more complex than just having a function. |
Why not? The serialised format can contain instance-level information so the deserialiser can recreate them appropriately. |
|
How would the deserializer have access to the arbitrary subclass for the RunStrategy object to be able to instantiate it? I would love to build it the way you're suggesting, so if there's an answer, I'm all ears. |
|
I assume they also need to be registered like timetable classes, since a custom RunStrategy is, similar to timetables, arbitrary code in the scheduler, and we need to safe-guard the invocation. (With the introduction of RunStrategy classes though, I’m thinking we could probably get rid of timetable registration if a timetable is composed solely from RunStrategy.) |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions. |
github-actions
bot
added
the
stale
2 participants
Given that timetables vary across multiple dimensions, I believe they are a good candidate for composition over inheritance. For instance, the CronTriggerTimetable is great, but it's easy to imagine wanting a variety of different data intervals other than a fixed timedelta, and that will quickly lead to subclass explosion.
This PR attempts to make that an option. There is a significant constraint in that all Timetable classes must be either plugins or part of Airflow proper, and they have to be serializable.