[`airflow`] Add fix to remove deprecated keyword arguments (`AIR302`) #14887

tirkarthi · 2024-12-10T06:13:35Z

Summary

Add replacement fixes to deprecated arguments of a DAG.

Ref #14582 #14626

Test Plan

Diff was verified and snapshots were updated.

tirkarthi · 2024-12-10T06:14:02Z

cc: @uranusjr @Lee-W

github-actions · 2024-12-10T06:25:20Z

`ruff-ecosystem` results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+3 -3 violations, +0 -0 fixes in 1 projects; 54 projects unchanged)

apache/airflow (+3 -3 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview --select ALL

+ dev/perf/dags/perf_dag_1.py:41:10: AIR302 `airflow.operators.bash_operator.BashOperator` is removed in Airflow 3.0
- dev/perf/dags/perf_dag_1.py:41:10: AIR302 `airflow.operators.bash_operator.BashOperator` is removed in Airflow 3.0; use `airflow.operators.bash.BashOperator` instead
+ dev/perf/dags/perf_dag_1.py:48:12: AIR302 `airflow.operators.bash_operator.BashOperator` is removed in Airflow 3.0
- dev/perf/dags/perf_dag_1.py:48:12: AIR302 `airflow.operators.bash_operator.BashOperator` is removed in Airflow 3.0; use `airflow.operators.bash.BashOperator` instead
+ performance/src/performance_dags/performance_dag/performance_dag.py:244:9: AIR302 [*] `schedule_interval` is removed in Airflow 3.0
- performance/src/performance_dags/performance_dag/performance_dag.py:244:9: AIR302 `schedule_interval` is removed in Airflow 3.0; use `schedule` instead

Changes by rule (1 rules affected)

code	total	+ violation	- violation	+ fix	- fix
AIR302	6	3	3	0	0

MichaReiser

Thank you

...r/src/rules/airflow/snapshots/ruff_linter__rules__airflow__tests__AIR302_AIR302_args.py.snap

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

MichaReiser · 2024-12-10T07:20:57Z

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

+    if let Some(ref mut diagnostic) = diagnostic {
+        diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement(
+            replacement?.to_string(),
+            diagnostic.range,
+        )));
+    }


We should expand the rule documentation and explain why the fix is unsafe.

The fixes are drop in replacements for keywords. I had them unsafe by default for review. Is there a convention over how to classify if a fix as safe or unsafe?

What Micha is referring to is to add a section like https://docs.astral.sh/ruff/rules/unused-import/#fix-safety that describes why the fix is marked as unsafe. You can use existing documentation as a reference, search for "Fix safety" at https://docs.astral.sh/ruff/rules.

I think the reason this is unsafe is because the user would still need to update the references to the argument in the function body.

Thanks for the link @dhruvmanila , I feel the fixes are safer in this case since the values to keyword arguments or the attributes are not really referenced anywhere. I have marked the fixes as safe.

keyword arguments or the attributes are not really referenced anywhere

I'm not sure I understand this, can you expand? The argument most likely is being used in the function body, I'm referring to those references.

The keyword arguments are to construction of a dag. Then the task constructed inside the context manager are automatically attached to the dag. They are not referred by the tasks in the context manager body and are specific to the dag object itself.

Below is a dag called latest_only where there are two tasks latest_only and task1 where task1 depends on latest_only and is denoted by overloading >> operator. Airflow earlier used to have schedule_interval and timetable but then they were merged to schedule keyword argument which is compatible to accept values and is a drop in replacement.

Before

with DAG( dag_id="latest_only", schedule_interval="daily", start_date=datetime.datetime(2021, 1, 1), catchup=False, tags=["example2", "example3"], sla_miss_callback=sla_callback ) as dag: latest_only = LatestOnlyOperator(task_id="latest_only") task1 = EmptyOperator(task_id="task1") latest_only >> task1

After fixes

with DAG( dag_id="latest_only", schedule="daily", start_date=datetime.datetime(2021, 1, 1), catchup=False, tags=["example2", "example3"], sla_miss_callback=sla_callback ) as dag: latest_only = LatestOnlyOperator(task_id="latest_only") task1 = EmptyOperator(task_id="task1") latest_only >> task1

Oh right, sorry I misunderstood. These are function call arguments and not function parameters. Yeah, I think these should be safe to fix.

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

dhruvmanila

Thank you for working on this! There are a few small changes but otherwise this looks good to go. Welcome to Ruff!

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

Co-authored-by: Dhruv Manilawala <[email protected]>

tirkarthi · 2024-12-10T14:21:33Z

Thanks @dhruvmanila and @MichaReiser for the review and merge.

* main: [`airflow`] Add fix to remove deprecated keyword arguments (`AIR302`) (#14887) Improve mdtests style (#14884) Reference `suppress-dummy-regex-options` in documentation of rules supporting it (#14888) [`flake8-bugbear`] `itertools.batched()` without explicit `strict` (`B911`) (#14408) [`ruff`] Mark autofix for `RUF052` as always unsafe (#14824) [red-knot] Improve type inference for except handlers (#14838) More typos found by codespell (#14880) [red-knot] move standalone expression_ty to TypeInferenceBuilder::file_expression_ty (#14879) [`ruff`] Do not simplify `round()` calls (`RUF046`) (#14832) Stop referring to early ruff versions (#14862) Fix a typo in `class.rs` (#14877) [`flake8-pyi`] Also remove `self` and `cls`'s annotation (`PYI034`) (#14801) [`pyupgrade`] Remove unreachable code in `UP015` implementation (#14871) [`flake8-bugbear`] Skip `B028` if `warnings.warn` is called with `*args` or `**kwargs` (#14870)

tirkarthi added 2 commits December 10, 2024 11:38

Add unsafe fixes based on suggestions.

aec2ce2

Fix formatting.

9de09c8

tirkarthi mentioned this pull request Dec 10, 2024

Airflow 3.0 deprecations #14626

Open

2 tasks

Fix clippy errors.

d8d8c71

MichaReiser added fixes Related to suggested fixes for violations preview Related to preview mode features labels Dec 10, 2024

MichaReiser reviewed Dec 10, 2024

View reviewed changes

dhruvmanila reviewed Dec 10, 2024

View reviewed changes

crates/ruff_linter/src/rules/airflow/rules/removal_in_3.rs Outdated Show resolved Hide resolved

dhruvmanila reviewed Dec 10, 2024

View reviewed changes

crates/ruff_linter/src/rules/airflow/rules/removal_in_3.rs Outdated Show resolved Hide resolved

dhruvmanila changed the title ~~[Airflow] Add automatic fixes to AIR302 argument rules~~ [airflow] Add unsafe fix for deprecated keyword arguments (AIR302) Dec 10, 2024

tirkarthi added 2 commits December 10, 2024 15:01

Refactor help and message string.

c6bb96b

Fix regression for sla_miss_callback.

d9b9a3b

dhruvmanila approved these changes Dec 10, 2024

View reviewed changes

crates/ruff_linter/src/rules/airflow/rules/removal_in_3.rs Outdated Show resolved Hide resolved

crates/ruff_linter/src/rules/airflow/rules/removal_in_3.rs Outdated Show resolved Hide resolved

crates/ruff_linter/src/rules/airflow/rules/removal_in_3.rs Outdated Show resolved Hide resolved

tirkarthi and others added 2 commits December 10, 2024 16:17

Update crates/ruff_linter/src/rules/airflow/rules/removal_in_3.rs

6a1c167

Co-authored-by: Dhruv Manilawala <[email protected]>

Mark the fixes as safe.

4862e4e

tirkarthi changed the title ~~[airflow] Add unsafe fix for deprecated keyword arguments (AIR302)~~ [airflow] Add fix for deprecated keyword arguments (AIR302) Dec 10, 2024

dhruvmanila changed the title ~~[airflow] Add fix for deprecated keyword arguments (AIR302)~~ [airflow] Add fix to remove deprecated keyword arguments (AIR302) Dec 10, 2024

dhruvmanila merged commit dc0d944 into astral-sh:main Dec 10, 2024
21 checks passed

BrewTestBot mentioned this pull request Dec 12, 2024

ruff 0.8.3 Homebrew/homebrew-core#200946

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[`airflow`] Add fix to remove deprecated keyword arguments (`AIR302`) #14887

[`airflow`] Add fix to remove deprecated keyword arguments (`AIR302`) #14887

tirkarthi commented Dec 10, 2024

tirkarthi commented Dec 10, 2024

github-actions bot commented Dec 10, 2024 •

edited

Loading

MichaReiser left a comment

MichaReiser Dec 10, 2024

tirkarthi Dec 10, 2024

dhruvmanila Dec 10, 2024

tirkarthi Dec 10, 2024

dhruvmanila Dec 10, 2024

tirkarthi Dec 10, 2024 •

edited

Loading

dhruvmanila Dec 10, 2024

dhruvmanila left a comment

tirkarthi commented Dec 10, 2024

[airflow] Add fix to remove deprecated keyword arguments (AIR302) #14887

[airflow] Add fix to remove deprecated keyword arguments (AIR302) #14887

Conversation

tirkarthi commented Dec 10, 2024

Summary

Test Plan

tirkarthi commented Dec 10, 2024

github-actions bot commented Dec 10, 2024 • edited Loading

ruff-ecosystem results

Linter (stable)

Linter (preview)

MichaReiser left a comment

Choose a reason for hiding this comment

MichaReiser Dec 10, 2024

Choose a reason for hiding this comment

tirkarthi Dec 10, 2024

Choose a reason for hiding this comment

dhruvmanila Dec 10, 2024

Choose a reason for hiding this comment

tirkarthi Dec 10, 2024

Choose a reason for hiding this comment

dhruvmanila Dec 10, 2024

Choose a reason for hiding this comment

tirkarthi Dec 10, 2024 • edited Loading

Choose a reason for hiding this comment

dhruvmanila Dec 10, 2024

Choose a reason for hiding this comment

dhruvmanila left a comment

Choose a reason for hiding this comment

tirkarthi commented Dec 10, 2024

[`airflow`] Add fix to remove deprecated keyword arguments (`AIR302`) #14887

[`airflow`] Add fix to remove deprecated keyword arguments (`AIR302`) #14887

github-actions bot commented Dec 10, 2024 •

edited

Loading

`ruff-ecosystem` results

tirkarthi Dec 10, 2024 •

edited

Loading