[airflow] Add fix to remove deprecated keyword arguments (AIR302)
#14887
Conversation
|
| code | total | + violation | - violation | + fix | - fix |
|---|---|---|---|---|---|
| AIR302 | 6 | 3 | 3 | 0 | 0 |
MichaReiser
added
fixes
...r/src/rules/airflow/snapshots/ruff_linter__rules__airflow__tests__AIR302_AIR302_args.py.snap
Outdated
Show resolved
Hide resolved
| if let Some(ref mut diagnostic) = diagnostic { | ||
| diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement( | ||
| replacement?.to_string(), | ||
| diagnostic.range, | ||
| ))); | ||
| } |
There was a problem hiding this comment.
We should expand the rule documentation and explain why the fix is unsafe.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 >> task1After 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 >> task1There was a problem hiding this comment.
Oh right, sorry I misunderstood. These are function call arguments and not function parameters. Yeah, I think these should be safe to fix.
dhruvmanila
changed the title
airflow] Add unsafe fix for deprecated keyword arguments (AIR302)
Co-authored-by: Dhruv Manilawala <dhruvmanila@gmail.com>
tirkarthi
changed the title
airflow] Add unsafe fix for deprecated keyword arguments (AIR302)airflow] Add fix for deprecated keyword arguments (AIR302)
dhruvmanila
changed the title
airflow] Add fix for deprecated keyword arguments (AIR302)airflow] Add fix to remove deprecated keyword arguments (AIR302)
|
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)
Summary
Add replacement fixes to deprecated arguments of a DAG.
Ref #14582 #14626
Test Plan
Diff was verified and snapshots were updated.