-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
[airflow] Avoid implicit DAG schedule (AIR301) #14581
[airflow] Avoid implicit DAG schedule (AIR301) #14581
Conversation
Airflow 3.0 changes the default of the 'schedule' argument. This causes incompatibility if a DAG previously does not specify a schedule explicitly. This is against best practice anyway---you should always prefer to set a schedule explicitly. Due to backward compatibility, Airflow 2 also possesses multiple arguments to set the schedule. These are all being combined into 'schedule' in 3.0. Usages of those old arguments are also detected.
|
code | total | + violation | - violation | + fix | - fix |
---|---|---|---|---|---|
AIR301 | 35 | 35 | 0 | 0 | 0 |
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.
This overall looks good to me, but I think we have to figure out the right scoping for all the Airflow3 deprecation rules.
crates/ruff_linter/src/rules/airflow/rules/dag_schedule_argument.rs
Outdated
Show resolved
Hide resolved
/// almost never what a user is looking for. Airflow 3 changes this the default | ||
/// to *None*, and would break existing DAGs using the implicit default. |
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.
Can we tell a bit more about how it breaks existing DAGS that use the implicit default. Will it fail with an exception or is it just that the default is different?
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.
Just the default is different—your DAGs will simply stop doing anything silently. I’ll add a paragraph to call this out.
crates/ruff_linter/src/rules/airflow/rules/dag_schedule_argument.rs
Outdated
Show resolved
Hide resolved
crates/ruff_linter/src/rules/airflow/rules/dag_schedule_argument.rs
Outdated
Show resolved
Hide resolved
4e33f61
to
7745a16
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.
This is great. Thank you :)
Summary
Airflow 3.0 changes the default of the 'schedule' argument. This causes incompatibility if a DAG previously does not specify a schedule explicitly. This is against best practice anyway---you should always prefer to set a schedule explicitly.
Due to backward compatibility, Airflow 2 also possesses multiple arguments to set the schedule. These are all being combined into 'schedule' in 3.0. Usages of those old arguments are also detected.
Test Plan
A test fixture has been included for the rule.