Skip to content

Commit e6a2de3

Browse files
authored
[airflow] Extract AIR311 from AIR301 rules (AIR301, AIR311) (#17310)
<!-- Thank you for contributing to Ruff! To help us out with reviewing, please consider the following: - Does this pull request include a summary of the change? (See below.) - Does this pull request include a descriptive title? - Does this pull request include references to any relevant issues? --> ## Summary <!-- What's the purpose of the change? What does it do, and why? --> As discussed in #14626 (comment), we're to separate suggested changes from required changes. The following symbols have been moved to AIR311 from AIR301. They still work in Airflow 3.0, but they're suggested to be changed as they're expected to be removed in a future version. * arguments * `airflow..DAG | dag` * `sla_miss_callback` * operators * `sla` * name * `airflow.Dataset] | [airflow.datasets.Dataset` → `airflow.sdk.Asset` * `airflow.datasets, rest @ ..` * `DatasetAlias` → `airflow.sdk.AssetAlias` * `DatasetAll` → `airflow.sdk.AssetAll` * `DatasetAny` → `airflow.sdk.AssetAny` * `expand_alias_to_datasets` → `airflow.sdk.expand_alias_to_assets` * `metadata.Metadata` → `airflow.sdk.Metadata` <!--airflow.models.baseoperator--> * `airflow.models.baseoperator.chain` → `airflow.sdk.chain` * `airflow.models.baseoperator.chain_linear` → `airflow.sdk.chain_linear` * `airflow.models.baseoperator.cross_downstream` → `airflow.sdk.cross_downstream` * `airflow.models.baseoperatorlink.BaseOperatorLink` → `airflow.sdk.definitions.baseoperatorlink.BaseOperatorLink` * `airflow.timetables, rest @ ..` * `datasets.DatasetOrTimeSchedule` → * `airflow.timetables.assets.AssetOrTimeSchedule` * `airflow.utils, rest @ ..` <!--airflow.utils.dag_parsing_context--> * `dag_parsing_context.get_parsing_context` → `airflow.sdk.get_parsing_context` ## Test Plan <!-- How was it tested? --> The test fixture has been updated acccordingly
1 parent c7b5067 commit e6a2de3

17 files changed

+1148
-958
lines changed

crates/ruff_linter/resources/test/fixtures/airflow/AIR301_args.py

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,6 @@
2222
DAG(dag_id="class_timetable", timetable=NullTimetable())
2323

2424

25-
def sla_callback(*arg, **kwargs):
26-
pass
27-
28-
29-
DAG(dag_id="class_sla_callback", sla_miss_callback=sla_callback)
30-
3125
DAG(dag_id="class_fail_stop", fail_stop=True)
3226

3327
DAG(dag_id="class_default_view", default_view="dag_default_view")
@@ -53,11 +47,6 @@ def decorator_timetable():
5347
pass
5448

5549

56-
@dag(sla_miss_callback=sla_callback)
57-
def decorator_sla_callback():
58-
pass
59-
60-
6150
@dag()
6251
def decorator_deprecated_operator_args():
6352
trigger_dagrun_op = trigger_dagrun.TriggerDagRunOperator(

crates/ruff_linter/resources/test/fixtures/airflow/AIR301_names.py

Lines changed: 2 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,6 @@
99
PY311,
1010
PY312,
1111
)
12-
from airflow import (
13-
Dataset as DatasetFromRoot,
14-
)
1512
from airflow.api_connexion.security import requires_access, requires_access_dataset
1613
from airflow.auth.managers.base_auth_manager import is_authorized_dataset
1714
from airflow.auth.managers.models.resource_details import DatasetDetails
@@ -26,25 +23,16 @@
2623
set,
2724
)
2825
from airflow.contrib.aws_athena_hook import AWSAthenaHook
29-
from airflow.datasets import (
30-
Dataset,
31-
DatasetAlias,
32-
DatasetAliasEvent,
33-
DatasetAll,
34-
DatasetAny,
35-
expand_alias_to_datasets,
36-
)
26+
from airflow.datasets import DatasetAliasEvent
3727
from airflow.datasets.manager import (
3828
DatasetManager,
3929
dataset_manager,
4030
resolve_dataset_manager,
4131
)
42-
from airflow.datasets.metadata import Metadata
4332
from airflow.hooks.base_hook import BaseHook
4433
from airflow.lineage.hook import DatasetLineageInfo
4534
from airflow.listeners.spec.dataset import on_dataset_changed, on_dataset_created
4635
from airflow.metrics.validators import AllowListValidator, BlockListValidator
47-
from airflow.models.baseoperator import chain, chain_linear, cross_downstream
4836
from airflow.operators.subdag import SubDagOperator
4937
from airflow.providers.amazon.aws.auth_manager.avp.entities import AvpEntities
5038
from airflow.providers.amazon.aws.datasets import s3
@@ -61,12 +49,10 @@
6149
from airflow.secrets.local_filesystem import LocalFilesystemBackend, load_connections
6250
from airflow.security.permissions import RESOURCE_DATASET
6351
from airflow.sensors.base_sensor_operator import BaseSensorOperator
64-
from airflow.timetables.datasets import DatasetOrTimeSchedule
6552
from airflow.timetables.simple import DatasetTriggeredTimetable
6653
from airflow.triggers.external_task import TaskStateTrigger
6754
from airflow.utils import dates
6855
from airflow.utils.dag_cycle_tester import test_cycle
69-
from airflow.utils.dag_parsing_context import get_parsing_context
7056
from airflow.utils.dates import (
7157
date_range,
7258
datetime_to_nano,
@@ -105,14 +91,9 @@
10591
# airflow.contrib.*
10692
AWSAthenaHook()
10793

94+
10895
# airflow.datasets
109-
Dataset()
110-
DatasetAlias()
11196
DatasetAliasEvent()
112-
DatasetAll()
113-
DatasetAny()
114-
Metadata()
115-
expand_alias_to_datasets
11697

11798
# airflow.datasets.manager
11899
DatasetManager()
@@ -134,10 +115,6 @@
134115
BlockListValidator()
135116

136117

137-
# airflow.models.baseoperator
138-
chain, chain_linear, cross_downstream
139-
140-
141118
# airflow.operators.branch_operator
142119
BaseBranchOperator()
143120

@@ -207,7 +184,6 @@
207184

208185

209186
# airflow.timetables
210-
DatasetOrTimeSchedule()
211187
DatasetTriggeredTimetable()
212188

213189
# airflow.triggers.external_task
@@ -231,8 +207,6 @@
231207
# airflow.utils.dag_cycle_tester
232208
test_cycle
233209

234-
# airflow.utils.dag_parsing_context
235-
get_parsing_context
236210

237211
# airflow.utils.db
238212
create_session
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
from __future__ import annotations
2+
3+
from datetime import timedelta
4+
5+
from airflow import DAG, dag
6+
from airflow.operators.datetime import BranchDateTimeOperator
7+
8+
9+
def sla_callback(*arg, **kwargs):
10+
pass
11+
12+
13+
DAG(dag_id="class_sla_callback", sla_miss_callback=sla_callback)
14+
15+
16+
@dag(sla_miss_callback=sla_callback)
17+
def decorator_sla_callback():
18+
pass
19+
20+
21+
@dag()
22+
def decorator_deprecated_operator_args():
23+
branch_dt_op2 = BranchDateTimeOperator(
24+
task_id="branch_dt_op2",
25+
sla=timedelta(seconds=10),
26+
)
27+
28+
branch_dt_op2
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
from __future__ import annotations
2+
3+
from airflow import Dataset as DatasetFromRoot
4+
from airflow.datasets import (
5+
Dataset,
6+
DatasetAlias,
7+
DatasetAll,
8+
DatasetAny,
9+
expand_alias_to_datasets,
10+
)
11+
from airflow.datasets.metadata import Metadata
12+
from airflow.models.baseoperator import chain, chain_linear, cross_downstream
13+
from airflow.models.baseoperatorlink import BaseOperatorLink
14+
from airflow.timetables.datasets import DatasetOrTimeSchedule
15+
from airflow.utils.dag_parsing_context import get_parsing_context
16+
17+
DatasetFromRoot()
18+
19+
# airflow.datasets
20+
Dataset()
21+
DatasetAlias()
22+
DatasetAll()
23+
DatasetAny()
24+
Metadata()
25+
expand_alias_to_datasets()
26+
27+
# airflow.models.baseoperator
28+
chain()
29+
chain_linear()
30+
cross_downstream()
31+
32+
# airflow.models.baseoperatolinker
33+
BaseOperatorLink()
34+
35+
# airflow.timetables.datasets
36+
DatasetOrTimeSchedule()
37+
38+
# airflow.utils.dag_parsing_context
39+
get_parsing_context()

crates/ruff_linter/src/checkers/ast/analyze/expression.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,9 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) {
229229
if checker.enabled(Rule::Airflow3Removal) {
230230
airflow::rules::airflow_3_removal_expr(checker, expr);
231231
}
232+
if checker.enabled(Rule::Airflow3SuggestedUpdate) {
233+
airflow::rules::airflow_3_0_suggested_update_expr(checker, expr);
234+
}
232235
if checker.enabled(Rule::Airflow3MovedToProvider) {
233236
airflow::rules::moved_to_provider_in_3(checker, expr);
234237
}
@@ -451,6 +454,9 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) {
451454
if checker.enabled(Rule::Airflow3Removal) {
452455
airflow::rules::airflow_3_removal_expr(checker, expr);
453456
}
457+
if checker.enabled(Rule::Airflow3SuggestedUpdate) {
458+
airflow::rules::airflow_3_0_suggested_update_expr(checker, expr);
459+
}
454460
if checker.enabled(Rule::Airflow3MovedToProvider) {
455461
airflow::rules::moved_to_provider_in_3(checker, expr);
456462
}
@@ -1152,6 +1158,9 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) {
11521158
if checker.enabled(Rule::Airflow3Removal) {
11531159
airflow::rules::airflow_3_removal_expr(checker, expr);
11541160
}
1161+
if checker.enabled(Rule::Airflow3SuggestedUpdate) {
1162+
airflow::rules::airflow_3_0_suggested_update_expr(checker, expr);
1163+
}
11551164
if checker.enabled(Rule::UnnecessaryCastToInt) {
11561165
ruff::rules::unnecessary_cast_to_int(checker, call);
11571166
}

crates/ruff_linter/src/codes.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1072,6 +1072,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
10721072
(Airflow, "002") => (RuleGroup::Preview, rules::airflow::rules::AirflowDagNoScheduleArgument),
10731073
(Airflow, "301") => (RuleGroup::Preview, rules::airflow::rules::Airflow3Removal),
10741074
(Airflow, "302") => (RuleGroup::Preview, rules::airflow::rules::Airflow3MovedToProvider),
1075+
(Airflow, "311") => (RuleGroup::Preview, rules::airflow::rules::Airflow3SuggestedUpdate),
10751076
(Airflow, "312") => (RuleGroup::Preview, rules::airflow::rules::Airflow3SuggestedToMoveToProvider),
10761077

10771078
// perflint

crates/ruff_linter/src/rules/airflow/helpers.rs

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,3 +81,45 @@ fn try_block_contains_undeprecated_import(try_node: &StmtTry, replacement: &Repl
8181
import_searcher.visit_body(&try_node.body);
8282
import_searcher.found_import
8383
}
84+
85+
/// Check whether the segments corresponding to the fully qualified name points to a symbol that's
86+
/// either a builtin or coming from one of the providers in Airflow.
87+
///
88+
/// The pattern it looks for are:
89+
/// - `airflow.providers.**.<module>.**.*<symbol_suffix>` for providers
90+
/// - `airflow.<module>.**.*<symbol_suffix>` for builtins
91+
///
92+
/// where `**` is one or more segments separated by a dot, and `*` is one or more characters.
93+
///
94+
/// Examples for the above patterns:
95+
/// - `airflow.providers.google.cloud.secrets.secret_manager.CloudSecretManagerBackend` (provider)
96+
/// - `airflow.secrets.base_secrets.BaseSecretsBackend` (builtin)
97+
pub(crate) fn is_airflow_builtin_or_provider(
98+
segments: &[&str],
99+
module: &str,
100+
symbol_suffix: &str,
101+
) -> bool {
102+
match segments {
103+
["airflow", "providers", rest @ ..] => {
104+
if let (Some(pos), Some(last_element)) =
105+
(rest.iter().position(|&s| s == module), rest.last())
106+
{
107+
// Check that the module is not the last element i.e., there's a symbol that's
108+
// being used from the `module` that ends with `symbol_suffix`.
109+
pos + 1 < rest.len() && last_element.ends_with(symbol_suffix)
110+
} else {
111+
false
112+
}
113+
}
114+
115+
["airflow", first, rest @ ..] => {
116+
if let Some(last) = rest.last() {
117+
*first == module && last.ends_with(symbol_suffix)
118+
} else {
119+
false
120+
}
121+
}
122+
123+
_ => false,
124+
}
125+
}

crates/ruff_linter/src/rules/airflow/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ mod tests {
2222
#[test_case(Rule::Airflow3Removal, Path::new("AIR301_airflow_plugin.py"))]
2323
#[test_case(Rule::Airflow3Removal, Path::new("AIR301_context.py"))]
2424
#[test_case(Rule::Airflow3MovedToProvider, Path::new("AIR302.py"))]
25+
#[test_case(Rule::Airflow3SuggestedUpdate, Path::new("AIR311_args.py"))]
26+
#[test_case(Rule::Airflow3SuggestedUpdate, Path::new("AIR311_names.py"))]
2527
#[test_case(Rule::Airflow3SuggestedToMoveToProvider, Path::new("AIR312.py"))]
2628
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
2729
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());

crates/ruff_linter/src/rules/airflow/rules/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,12 @@ pub(crate) use dag_schedule_argument::*;
22
pub(crate) use moved_to_provider_in_3::*;
33
pub(crate) use removal_in_3::*;
44
pub(crate) use suggested_to_move_to_provider_in_3::*;
5+
pub(crate) use suggested_to_update_3_0::*;
56
pub(crate) use task_variable_name::*;
67

78
mod dag_schedule_argument;
89
mod moved_to_provider_in_3;
910
mod removal_in_3;
1011
mod suggested_to_move_to_provider_in_3;
12+
mod suggested_to_update_3_0;
1113
mod task_variable_name;

0 commit comments

Comments
 (0)