Skip to content
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] rule for deprecated task_concurrency parameter (AIR303) #14616

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

abhishekbhakat
Copy link

@abhishekbhakat abhishekbhakat commented Nov 26, 2024

Summary

Airflow 3.0 renames the task_concurrency parameter to max_active_tis_per_dag across all operators. The old parameter will no longer have any effect in 3.0. This rule detects usage of the deprecated parameter to help users migrate their DAGs to the new concurrency control pattern.

Ref: #14626

Test Plan

A test fixture has been included for the rule.

@MichaReiser
Copy link
Member

So many airflow rules :)

Are you working/coordinating with @uranusjr ? I just want to ensure we have "one" authority that has a big picture in mind for all tne new airflow rules.

@MichaReiser MichaReiser added rule Implementing or modifying a lint rule preview Related to preview mode features labels Nov 26, 2024
Copy link
Contributor

github-actions bot commented Nov 26, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@abhishekbhakat
Copy link
Author

Yes and a whole lot more coming.

@abhishekbhakat abhishekbhakat marked this pull request as draft November 26, 2024 18:50
@abhishekbhakat abhishekbhakat changed the title rule for deprecated task_concurrency parameter [airflow] rule for deprecated task_concurrency parameter (AIR303) Nov 26, 2024
@abhishekbhakat abhishekbhakat marked this pull request as ready for review November 27, 2024 05:32
@dhruvmanila
Copy link
Member

Are there other parameters that have been deprecated? I wonder if we should club them under a single rule similar to the NumPy 2.0 deprecations where one rule would look at the imports and the other would look at function parameters.

@abhishekbhakat
Copy link
Author

There are a lot of breaking changes in Airflow 3 coming from Airflow 2.
Here is an exhaustive list apache/airflow#41641
We are picking one by one. Will try to club them wherever possible.

- Add new rule to detect deprecated task_concurrency parameter
- Update documentation and tests
- Add snapshot tests
@MichaReiser
Copy link
Member

Oh wow, that's a very long list. I didn't expect there to be that many changes.

I think it would be great to write up a short proposal on what rules you plan on adding and how you want to group the deprecations. We want to avoid having 50ish airflow deprecation rules. For example: Can we have one rule for:

  • removals (with a possible fix suggesting the new name)
  • deprecations
  • new defaults (or similar)

We can incrementally add to those rules.

I think a good place to discuss the new rules is #14626

# Using deprecated task_concurrency parameter
task1 = PythonOperator(
task_id="task1",
task_concurrency=2, # This should trigger AIR303
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@abhishekbhakat We should raise a deprecation error in Airflow 2.2+ too: apache/airflow#17708 (that was when we had original deprecation)

In Airflow 3, it is "removed" entirely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
preview Related to preview mode features rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants