Hi all,

While fixing `airflow config get-value` (PR #72145
<https://github.com/apache/airflow/pull/72145#issuecomment-5438953827>) I
was asked to check
the wider picture before changing something that is part of the Public
Interface. Here is what I found.

## The observation

For commands that take an identifier, look one thing up and print it to
stdout -- the ones people wrap in `v=$(airflow ...)` -- "target does not
exist" is handled inconsistently. Measured on main, in Breeze, against a
migrated metadata DB:

  command (target missing)         exit  stdout     stderr
  -------------------------------  ----  ---------  -------------------
  variables get                      1   (empty)    clean message
  pools get                          1   (empty)    clean message
  connections get                    1   (empty)    clean message
  providers get                      1   (empty)    clean message
  dags details                       1   (empty)    clean message
  assets details                     1   (empty)    clean message
  dags state (dag missing)           1   (empty)    clean message
  tasks states-for-dag-run           1   (empty)    1126-byte traceback
  dags state (dag ok, run gone)      0   "None"     (empty)
  config get-value                   0   (nothing)  (nothing)

Two things stand out:

1. `config get-value` exits 0 and prints nothing at all, so a caller
   cannot tell a missing option from an option set to the empty string.
   It also let the config parser's "not found" warning reach *stdout*,
   corrupting the one stream the command exists to produce.

2. `airflow dags state <dag> <bad-run-id>` prints the literal string
   "None" and exits 0. git log suggests this was never a decision:
   #21793 (2022) added a SystemExit for the missing-DAG case in the same
   function but left this branch alone, and #46407 (2025) mechanically
   preserved it while refactoring for null logical dates. It looks like
   the accidental output of str(None).

## The convention already exists

- The Stable REST API returns 404 "Option [section/option] not found."
  for the config case.
- `airflowctl config get` is generated over that API, so it already
  surfaces the error.
- 8 of the 10 cases above already exit non-zero with a clean message on
  stderr.

So this is not a proposal for a new convention. It is a proposal to
finish an existing one.

## Why now rather than later

contributing-docs/27_cli_implementation_guide.rst states that under
AIP-94 the existing `airflow` CLI remote commands stay in place but get
rewired internally to call the Public API. When `config get-value` is
rewired, it will inherit the API's 404 and stop exiting 0 regardless of
what we decide here.

That is really what I would like input on: this behaviour change is
already coming. Is it better to make it deliberately now, with a
newsfragment, in a small PR people can find -- or to let it arrive as a
side effect of the rewiring, where it is much easier to miss?

## What I would propose

1. `config get-value`: exit non-zero with a message on stderr when the
   option does not exist, and keep stdout clean. (PR #72145, open.)
2. `dags state`: same treatment for the missing-DagRun branch, instead
   of printing "None".
3. A newsfragment for both, since scripts may depend on the current
   exit status.
4. Separately and cosmetically, `tasks states-for-dag-run` could raise
   SystemExit rather than letting DagRunNotFound reach the user as a
   traceback. Different problem, happy to leave it out of scope.

Note that public-airflow-interface.rst already says the CLI "behaviour
might change in details" and recommends the REST API for programmatic
use, so I do not think this needs a deprecation cycle -- but I would
rather hear that from the list than assume it.

Does aligning these two look right, and is doing it deliberately now
preferable to letting AIP-94 do it silently later?

Thanks,
Eason

Reply via email to