Skip to content

[AIP-94] Route airflow dags list through the API server#68444

Open
henry3260 wants to merge 1 commit into
apache:mainfrom
henry3260:migrate-airflowctl-dags-list
Open

[AIP-94] Route airflow dags list through the API server#68444
henry3260 wants to merge 1 commit into
apache:mainfrom
henry3260:migrate-airflowctl-dags-list

Conversation

@henry3260

@henry3260 henry3260 commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

What

  • Migrated airflow dags list to fetch Dags through the typed airflowctl API client instead of accessing serialized Dags and import errors directly from the metadata database.
  • Added a deprecation warning directing users to airflowctl dags list.

related: #68402


Was generative AI tooling used to co-author this PR?
  • Yes (please specify the tool below)

  • Read the Pull Request Guidelines for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
  • For fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
  • When adding dependency, check compliance with the ASF 3rd Party License Policy.
  • For significant user-facing changes create newsfragment: {pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.

@ashb ashb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

  1. If airflowctl exists, why change this CLI at all. Deprecate it sure, but why change it to use the API endpoint which may or may not be running when?
  2. Please improve the commit message as per our contributing guidlines
  3. Where does it get auth from?

@henry3260

Copy link
Copy Markdown
Contributor Author
  1. If airflowctl exists, why change this CLI at all. Deprecate it sure, but why change it to use the API endpoint which may or may not be running when?
  2. Please improve the commit message as per our contributing guidlines
  3. Where does it get auth from?

It gets authentication from AIRFLOW_CLI_TOKEN when that environment variable
is set. Otherwise, it follows the design introduced in #68175: the locally
configured auth manager provides a CLI user and mints a short-lived JWT via
get_cli_user() and generate_jwt().

I interpreted #68402's instruction to follow #68175 as requiring the legacy
airflow command to use that API client as part of the migration.

…e metadata

database directly, while preserving the existing --local behavior.
@henry3260 henry3260 force-pushed the migrate-airflowctl-dags-list branch from fcdcfe9 to 2cb5c32 Compare June 12, 2026 18:22

@bugraoz93 bugraoz93 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should use airflowctl client to call any resource. It should be fetch from there
Example from issue #68175

Comment thread airflow-core/src/airflow/cli/commands/dag_command.py
@bugraoz93

Copy link
Copy Markdown
Contributor

@henry3260 could you please update PR and it's decriptions :)

  1. If airflowctl exists, why change this CLI at all. Deprecate it sure, but why change it to use the API endpoint which may or may not be running when?
  2. Please improve the commit message as per our contributing guidlines
  3. Where does it get auth from?

Hey Ash, thanks for raising this! There will be some PRs like this using airflowctl client. I added the auth and how the client should be created here #68175. It should use that.
This work should use airflowctl behind the scenes as we agreed under AIP-94. We agreed on using the airflowctl client, so behind the scenes the CLI will work by generating the API token if it is running on the server as an admin for those commands, or by using the proper config embedded locally. Otherwise, we should direct people to use airflowctl directly, as they can achieve the same results with it safely with RBAC.
TLDR
Normally, the CLI requires direct DB credentials to run, so if the auth configuration is present, it should work as expected since it should already exist on the server. Given that we already expose DB credentials, and with FAB even user credentials using DB ones, this should not introduce any additional exposure. We expect the auth-related configuration to be available in airflow.cfg when invoking the CLI, as it is already required for Airflow to run (on an instance from admin to execute).

https://github.com/apache/airflow/blob/main/airflow-core/src/airflow/cli/api_client.py

@bugraoz93 bugraoz93 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good Henry!

Hey Ash, could you please share your concern about the approach please? I would be happy to hear and make it better if we can at any point

@bugraoz93 bugraoz93 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

On second thought, we don't need this change at all. We can just redirect it with a warning. It will only create maintenance here with the additional code we are adding. Just adding a deprecation warning should be enough for this command, as Ash suggested.
Edit: I missed the SELECT statement we deleted and replaced with airflowctl. To be honest, the GitHub UI tricked me the more I looked, as I pulled it locally to really see. My above comment applies 😅 I thought that local and what we are doing for actual parsing is the same. The deletion was hidden between additions
https://github.com/apache/airflow/pull/68444/changes#diff-b28357c8748ec828697a10b2d785bffea845397916d410da18e0265b54155867L532-L536
This part indeed should be replaced with the airflowctl client as described in the issue and in AIP-94. What we need to ensure while deprecating, airflowctl should also have --local support before Airflow v4.

@bugraoz93

bugraoz93 commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

@henry3260 if we only used airflowctl, the changes would make sense. We should just add a deprecation warning in this case. It was my bad to miss this part when creating the issue.

I will update the issue by checking this case for commands and adding a (deprecation only) note on the side

@henry3260

Copy link
Copy Markdown
Contributor Author

On second thought, we don't need this change at all. We can just redirect it with a warning. It will only create maintenance here with the additional code we are adding. Just adding a deprecation warning should be enough for this command, as Ash suggested. Edit: I missed the SELECT statement we deleted and replaced with airflowctl. To be honest, the GitHub UI tricked me the more I looked, as I pulled it locally to really see. My above comment applies 😅 I thought that local and what we are doing for actual parsing is the same. The deletion was hidden between additions https://github.com/apache/airflow/pull/68444/changes#diff-b28357c8748ec828697a10b2d785bffea845397916d410da18e0265b54155867L532-L536 This part indeed should be replaced with the airflowctl client as described in the issue and in AIP-94. What we need to ensure while deprecating, airflowctl should also have --local support before Airflow v4.

Haha, no worries! The GitHub UI has definitely tricked me before too. 😅

@ashb

ashb commented Jun 16, 2026

Copy link
Copy Markdown
Member

As I mentioned to Buğra on slack, my view on this is: I don't think we should be using the client. Either we should leave it exactly as it is (i.e. just issue a deprecation warning but otherwise change nothing) or, based on

image

I would expect us to call something like calling airflowctl.ctl.commands.dag_command.list -- however I don't even see a list command!

@potiuk potiuk added the ready for maintainer review Set after triaging when all criteria pass. label Jun 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:CLI ready for maintainer review Set after triaging when all criteria pass.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants