-
Notifications
You must be signed in to change notification settings - Fork 14.4k
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
Unify DAG schedule args and change default to None #41453
Conversation
961462c
to
53391bd
Compare
2ab573d
to
f8202f8
Compare
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.
One small nitpick. Everything else looks good to me, but I'm not an expert in the area. It would be great to have a second set of eyes.
if we're to add migration rule for this PR to #41641, we probably could do the following 2?
|
These two are sort of interchangable for display purposes, but not if you want to use the value for something else. For example: with DAG(schedule=timedelta(days=2)) as d1:
...
# This can not be changed to timetable_summary.
# d1.timetable would work, but fail for other use cases.
with DAG(schedule=d1.schedule_interval) as d2:
... If we’re going the linter (ruff-like) route, this would be a case for showing a linting error without an automated fix available.
|
95b64ea
to
91e5d67
Compare
The arguments 'schedule_interval' and 'timetable' are removed from both the DAG class and the `@dag` decorator. The default value of the 'schedule' argument (on both entities) is changed to None (i.e. a DAG will not have a schedule by default). The 'timetable' attribute still exists on DAG, and is now the only value that reflects the DAG's schedule. The 'schedule_interval' attribute is removed from DAG. The 'schedule_interval' on DagModel used to store a string representation of DAG's attribute of the same name, is now replaced by timetable_summary, which should (mostly?) work the same as before. We can fix minor UI differences as we go. Some use cases that rely on that field are also changed to use other fields instead (dataset_expression, for example, can be used to check whether a DAG is dataset-triggered). The API field 'schedule_interval' has also been removed since that field no longer exists in the database. This has some side effects. The field previously contains some type information for delta values, but now becomes a simple string. It is unclear if anyone really needs the information, but we can always bring it back if needed.
The key in facets are kept for backward compatibility, but they now do not produce a value in the resulting JSON. A new key timetable_summary is added on Airflow 3 to replace the old key, similar to how it's done in the REST API.
91e5d67
to
c1b2265
Compare
The arguments 'schedule_interval' and 'timetable' are removed from both the DAG class and the
@dag
decorator.The default value of the 'schedule' argument (on both entities) is changed to None (i.e. a DAG will not have a schedule by default).
The 'timetable' attribute still exists on DAG, and is now the only value that reflects the DAG's schedule. The 'schedule_interval' attribute is removed from DAG.
The 'schedule_interval' on DagModel used to store a string representation of DAG's attribute of the same name, is now replaced by timetable_summary, which should (mostly?) work the same as before. We can fix minor UI differences as we go. Some use cases that rely on that field are also changed to use other fields instead (dataset_expression, for example, can be used to check whether a DAG is dataset-triggered).
The API field 'schedule_interval' has also been removed since that field no longer exists in the database. This has some side effects. The field previously contains some type information for delta values, but now becomes a simple string. It is unclear if anyone really needs the information, but we can always bring it back if needed.
Close #24842