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.

Sign up for GitHub

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

Jump to bottom

Add airflow config lint cli command for lint the configuration changes from Airflow 2.x to Airflow 3.0 #44908

Merged
merged 6 commits into from
Dec 19, 2024
Merged

Add airflow config lint cli command for lint the configuration changes from Airflow 2.x to Airflow 3.0 #44908

merged 6 commits into from
Dec 19, 2024

Conversation

sunank200
Copy link
Collaborator

@sunank200 sunank200 commented Dec 13, 2024 •
edited
Loading

This PR introduces the airflow config lint command as part of the Airflow CLI config commands for migration tooling for Airflow 3.0. The purpose of this command is to help users transition from Airflow 2.x to 3.0 by identifying and providing actionable feedback on removed and renamed configuration parameters.

Implementation details:

  • Identify parameters that are no longer valid in Airflow 3.0 based on 44555
  • Highlights renamed parameters and provides guidance for transitioning to their new names and sections in the lint error.
  • Provider users options to specify sections, options, and ignore lists for granular control.

CLI Usage Examples:

  1. Lint all sections and options:

    airflow config lint

  2. Lint a specific sections:

    airflow config lint --section core,webserver

  3. Lint a specific sections and options:

    airflow config lint --section smtp --option smtp_user

  4. Ignore a specific sections during linting:

    airflow config lint --ignore-section webserver,api

  5. Ignore a specific options during linting:

    airflow config lint --ignore-option smtp_user,session_lifetime_days

  6. Enable verbose output for detailed feedback:

    airflow config lint --verbose

Following is the screenshot of the command used:
Screenshot 2024-12-17 at 2 56 15 AM

closes: #44555
related: #41641

^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

This was referenced Dec 13, 2024
Open
Closed
@sunank200 sunank200 marked this pull request as ready for review December 13, 2024 11:16
@sunank200 sunank200 self-assigned this Dec 13, 2024
@potiuk
Copy link
Member

potiuk commented Dec 13, 2024

NAAAAJS

amoghrajesh
amoghrajesh approved these changes Dec 13, 2024
View reviewed changes
Copy link
Contributor

@amoghrajesh amoghrajesh left a comment

Choose a reason for hiding this comment

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

Awesome, looks really nice!
@sunank200 one comment but thats just a thought. Will it also be possible to add a short README or so in order for someone to deprecate any future options? (Although i think all are done)

airflow/cli/commands/remote_commands/config_command.py Show resolved Hide resolved
gopidesupavan
gopidesupavan approved these changes Dec 13, 2024
View reviewed changes
Copy link
Member

@gopidesupavan gopidesupavan left a comment

Choose a reason for hiding this comment

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

Awesome :)

@sunank200
Copy link
Collaborator Author

Awesome, looks really nice! @sunank200 one comment but thats just a thought. Will it also be possible to add a short README or so in order for someone to deprecate any future options? (Although i think all are done)

Readme for in how to deprecate options in future? @amoghrajesh can you elaborate on this, please

@amoghrajesh
Copy link
Contributor

Awesome, looks really nice! @sunank200 one comment but thats just a thought. Will it also be possible to add a short README or so in order for someone to deprecate any future options? (Although i think all are done)

Readme for in how to deprecate options in future? @amoghrajesh can you elaborate on this, please

Yeah, that is what I was hinting at. But looking at the state of this PR, it is a good example for how to do it later in future. So I wouldnt ask for that anymore

@uranusjr
Copy link
Member

re: The README topic, I feel at least we should introduce a news fragment on this.

We should probably add a page in documentation that guides people through the migrations. This would be useful for us in blogposts and discussions on Airflow 2-to-3 upgrade path. (This should be a separate PR.)

uranusjr
uranusjr reviewed Dec 16, 2024
View reviewed changes
airflow/cli/cli_config.py Outdated Show resolved Hide resolved
uranusjr
uranusjr reviewed Dec 16, 2024
View reviewed changes
airflow/cli/cli_config.py Outdated Show resolved Hide resolved
@uranusjr uranusjr added the backport-to-v2-10-test Mark PR with this label to backport to v2-10-test branch label Dec 16, 2024
@sunank200 sunank200 force-pushed the airflow-config-lint branch 3 times, most recently from fba9885 to 15fe43b Compare December 16, 2024 21:06
@sunank200 sunank200 requested review from uranusjr and Lee-W December 16, 2024 21:09
@Lee-W
Copy link
Member

Lee-W commented Dec 17, 2024

one test case failed

Lee-W
Lee-W reviewed Dec 17, 2024
View reviewed changes
airflow/cli/commands/remote_commands/config_command.py Outdated Show resolved Hide resolved
airflow/cli/commands/remote_commands/config_command.py Outdated Show resolved Hide resolved
airflow/cli/commands/remote_commands/config_command.py Outdated Show resolved Hide resolved
newsfragments/44908.significant.rst Outdated Show resolved Hide resolved
@sunank200
Copy link
Collaborator Author

one test case failed

It was from different section and fixed it now.

@sunank200 sunank200 requested a review from Lee-W December 17, 2024 06:42
@sunank200 sunank200 force-pushed the airflow-config-lint branch 2 times, most recently from d53876a to 069b1b7 Compare December 17, 2024 06:46
Lee-W
Lee-W approved these changes Dec 17, 2024
View reviewed changes
Copy link
Member

@Lee-W Lee-W left a comment

Choose a reason for hiding this comment

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

Overall, looks good. Left 2 nits. I'm planning on merging it later tonight unless anyone wants to take a deeper look

tests/cli/commands/remote_commands/test_config_command.py Outdated Show resolved Hide resolved
tests/cli/commands/remote_commands/test_config_command.py Show resolved Hide resolved
@sunank200 sunank200 requested a review from kaxil December 17, 2024 14:38
@sunank200 sunank200 force-pushed the airflow-config-lint branch 2 times, most recently from c5f8519 to 6218af9 Compare December 17, 2024 15:07
@Lee-W
Copy link
Member

Lee-W commented Dec 18, 2024

@sunank200 One discussion has not yet been resolved. Could you please check whether that has been addressed? If so, I'll merge it. Thanks!

@Lee-W Lee-W merged commit bc813c6 into apache:main Dec 19, 2024
49 checks passed
@Lee-W Lee-W deleted the airflow-config-lint branch December 19, 2024 08:32
@github-actions GitHub Actions
Copy link

Backport failed to create: v2-10-test. View the failure log Run details

Status Branch Result
❌ v2-10-test Commit Link

You can attempt to backport this manually by running:

cherry_picker bc813c6 v2-10-test

This should apply the commit to the v2-10-test branch and leave the commit in conflict state marking
the files that need manual conflict resolution.

After you have resolved the conflicts, you can continue the backport process by running:

cherry_picker --continue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@amoghrajesh amoghrajesh amoghrajesh approved these changes

@potiuk potiuk potiuk approved these changes

@gopidesupavan gopidesupavan gopidesupavan approved these changes

@Lee-W Lee-W Lee-W approved these changes

@phanikumv phanikumv Awaiting requested review from phanikumv

@jedcunningham jedcunningham Awaiting requested review from jedcunningham

@ashb ashb Awaiting requested review from ashb

@cmarteepants cmarteepants Awaiting requested review from cmarteepants

@ephraimbuddy ephraimbuddy Awaiting requested review from ephraimbuddy

@uranusjr uranusjr Awaiting requested review from uranusjr

@kaxil kaxil Awaiting requested review from kaxil

Assignees

@sunank200 sunank200

Labels
area:CLI backport-to-v2-10-test Mark PR with this label to backport to v2-10-test branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Airflow 2 to 3 auto migration rules - airflow config
8 participants
@sunank200 @potiuk @amoghrajesh @uranusjr @Lee-W @ephraimbuddy @kaxil @gopidesupavan