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

AIP-81 Move CLI Commands to directories according to Hybrid, Local and Remote #44538

Merged
merged 5 commits into from
Dec 3, 2024

Conversation

bugraoz93
Copy link
Collaborator

closes: #44204


^ 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.

@boring-cyborg boring-cyborg bot added area:CLI area:dev-tools provider:cncf-kubernetes Kubernetes provider related issues labels Dec 1, 2024
@bugraoz93 bugraoz93 added the AIP-81 Enhanced Security in CLI via Integration of API label Dec 1, 2024
@jscheffl
Copy link
Contributor

jscheffl commented Dec 1, 2024

Good that you separate big re-factoring and shifting files. Let's see if CI turns green, then after short screening I have no issue for fast merge.

@bugraoz93
Copy link
Collaborator Author

Good that you separate big re-factoring and shifting files. Let's see if CI turns green, then after short screening I have no issue for fast merge.

Some turns out red :( I will check it out shortly

@bugraoz93
Copy link
Collaborator Author

I am surprised that my local pre-commit didn't catch this. They were there but weren't added to with git 😓

@bugraoz93 bugraoz93 force-pushed the feat/44204/split-cli-commands branch 2 times, most recently from 7473149 to 4f31c92 Compare December 1, 2024 20:42
Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

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

Nice!

@potiuk
Copy link
Member

potiuk commented Dec 3, 2024

You need to rebase / solve conflicts now @bugraoz93

@bugraoz93 bugraoz93 force-pushed the feat/44204/split-cli-commands branch from 4f31c92 to 89be8ed Compare December 3, 2024 16:29
@bugraoz93
Copy link
Collaborator Author

Rebased, thanks a lot for the reviews! ❤️

@potiuk potiuk merged commit 2b7015e into apache:main Dec 3, 2024
97 checks passed
@millin
Copy link
Contributor

millin commented Dec 27, 2024

This broke airflow 2.10 + apache-airflow-providers-celery 3.9.0

ModuleNotFoundError: No module named 'airflow.cli.commands.local_commands'

@bugraoz93
Copy link
Collaborator Author

This broke airflow 2.10 + apache-airflow-providers-celery 3.9.0

ModuleNotFoundError: No module named 'airflow.cli.commands.local_commands'

Thanks @millin for bringing this up!

@kurtrwall
Copy link

Thanks for the catch. When is this fix expected to be released?

LefterisXefteris pushed a commit to LefterisXefteris/airflow that referenced this pull request Jan 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AIP-81 Enhanced Security in CLI via Integration of API area:CLI area:dev-tools provider:cncf-kubernetes Kubernetes provider related issues
Development

Successfully merging this pull request may close these issues.

AIP-81 Split CLI Commands into Local, Remote and Hybrid Commands
5 participants