Eason09053360 opened a new pull request, #72677:
URL: https://github.com/apache/airflow/pull/72677
`airflow jobs check --allow-multiple --limit 0` always exits 1, even though
`0` is the documented
value for disabling the limit.
The guard rejected any limit at or below `1`:
```python
if args.allow_multiple and args.limit <= 1:
raise SystemExit("To use option --allow-multiple, you must set the limit
to a value greater than 1.")
```
Rejecting `1` is correct — asking to see multiple jobs while only inspecting
the single most recent
one is contradictory. But `0` means "no upper bound", which is the opposite
of contradictory, and it
got caught by the same `<=`.
Everything else in the codebase already treats `0` as valid:
| Where | What it says about `--limit 0` |
| --- | --- |
| `cli_config.py` — `ARG_JOB_LIMIT` | `type=positive_int(allow_zero=True)`,
so argparse accepts `0` |
| the flag's own `help` | "The number of recent jobs that will be checked.
To disable limit, set 0." |
| `jobs_command.check()`, a few lines below the guard | `if args.limit > 0:
query = query.limit(args.limit)` — already skips the `LIMIT` clause for `0` |
| `airflowctl jobs check` — the successor this command is
`@deprecated_for_airflowctl` in favour of | uses `args.limit == 1` and ships a
`--limit 0 --allow-multiple` test asserting success |
So the guard was the only place that disagreed, and it made the branch below
it unreachable.
## Impact
Running highly available schedulers, the natural way to ask "are all my
schedulers alive?" is:
```console
$ airflow jobs check --job-type SchedulerJob --allow-multiple --limit 0
To use option --allow-multiple, you must set the limit to a value greater
than 1. [rc=1]
```
The error message then points away from the value that would have worked, so
operators fall back to
a guessed upper bound such as `--limit 100` (the form the docs and the
command epilog show). That
under-reports once more jobs than the bound are running.
## Changes
- Narrow the guard to `args.limit == 1`. `positive_int(allow_zero=True)`
already rejects negatives,
so the remaining values are `0, 1, 2, …` and `== 1` is exact.
- Reword the error to mention `0`, matching `airflowctl jobs check` verbatim
so both CLIs emit the
same sentence.
- Document `--limit 0` next to the existing high-availability example in
`check-health.rst`. The
existing `--limit 100` example is left as-is.
## Tests
`test_should_report_success_for_ha_schedulers_with_limit_disabled` mirrors
the neighbouring
`--limit 100` test with `--limit 0`. Reverting the one-line guard change and
running the whole file
fails that test with the old `SystemExit`; with the change, all 7 tests in
the file pass.
The pre-existing
`test_should_raise_exception_for_allow_multiple_and_limit_1` still passes — the
contradictory `--limit 1` case is still rejected.
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes — Claude Code (Opus 5)
Generated-by: Claude Code (Opus 5) following [the
guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
