Hi all,
First of all, I’d like to wish everyone a Merry Christmas and a happy
holiday season 🎄!
I’d like to start a discussion about introducing a new `cli` section in
provider metadata, with the goals of:
1.
**Improving Airflow CLI startup and response time**
According to the PoC benchmark, this change provides a noticeable
performance improvement.
2.
**Unlocking the ability for all providers to expose commands in the
Airflow CLI**
Currently, only AuthManger and Executor can expose commands.
This change is probably not large enough to justify a full AIP, so I
believe a discussion followed by lazy consensus should be sufficient.
------------------------------
Why
Before the recent refactor, regardless of which `airflow` command is
executed, `cli_parser` imports the actual AuthManager and Executor in use
in order to call `get_cli_commands` and collect optional CLI commands [1].
This means that **every** CLI invocation — including something as simple as
`airflow --help` — will import heavy modules such as `kubernetes`, `
flask_appbuilder`, etc., depending on the values of `
AIRFLOW__CORE__AUTH_MANAGER` and `AIRFLOW__CORE__EXECUTOR`.
In the worst case (e.g. `FabAuthManager` + `CeleryKubernetesExecutor`), it
takes **~5 seconds** just to display `airflow --help` based on the
benchmark results.
------------------------------
How
The refactor includes:
1.
Adding a `cli` section to provider metadata (`provider.yaml` / `def
get_provider_info`) that points to `get_cli_commands`
2.
Moving `get_cli_commands` into a **clean** module that does not import
any heavy dependencies
-
It should only import from `airflow.cli.cli_config`
-
It should rely on `lazy_load_command`
------------------------------
What
The main behavioral change is that, after this refactor, **any installed
provider that exposes CLI commands will have those commands available in
the Airflow CLI**, even if it is not configured as the active AuthManager
or Executor.
For example:
-
If both the Celery and Kubernetes providers are installed
-
And `AIRFLOW__CORE__EXECUTOR=LocalExecutor`
The Celery and Kubernetes command groups will still appear in `airflow
--help`.
If there are no strong drawbacks to introducing the cli section in provider
metadata, I can either:
-
Break the change down provider by provider, or
-
Submit one larger atomic change covering all providers
------------------------------
PoC & Summary
I’ve completed a PoC [2] along with a benchmark script [3] and result [4].
Below is a summary of the CLI response time improvements:
-
*Overall average*: 3.117s, down from 4.048s (*~23.0% improvement*)
-
*Fastest run*: 3.092s, down from 3.566s (*~13.3% improvement*)
-
*Slowest run*: 3.155s, down from 5.006s (*~37.0% improvement*)
------------------------------
*References*
[1]
https://github.com/apache/airflow/blob/ac085a425652d16b5fff17f8e937938c7d47b868/airflow-core/src/airflow/cli/cli_parser.py#L62-L86
[2] https://github.com/apache/airflow/pull/59805
[3]
https://github.com/apache/airflow/pull/59805/changes#diff-7ef81cd1589183b63d2452f64809adcba5f6b14e5ee337be02524d83ad6698e4
[4] https://github.com/apache/airflow/pull/59805#benchmark-result
I’d really appreciate any feedback, suggestions, or concerns about this
approach. Thanks!
Best regards,
Jason
