roykoand commented on code in PR #73105:
URL: https://github.com/apache/airflow/pull/73105#discussion_r4002582007
##########
airflow-core/src/airflow/cli/cli_config.py:
##########
@@ -294,7 +294,7 @@ def string_lower_type(val):
# list_jobs
ARG_DAG_ID_OPT = Arg(("-d", "--dag-id"), help="The id of the dag")
-ARG_LIMIT = Arg(("--limit",), help="Return a limited number of records")
+ARG_LIMIT = Arg(("--limit",), type=positive_int(allow_zero=False),
help="Return a limited number of records")
Review Comment:
Good catch, thanks — fixed in 6943e16e90 by switching to `allow_zero=True`.
`--limit 0` is a legitimate prior invocation (`LIMIT 0`, a well-defined "return
no records" query), unlike `--limit -1`, whose behavior was
undefined/driver-specific and is still correctly rejected (`positive_int`'s
zero-allowance only special-cases `0`, negatives are always rejected regardless
of `allow_zero`). Added `test_cli_list_jobs_with_zero_limit` and
`test_cli_list_jobs_with_negative_limit` to cover both.
I kept the invalid-limit test asserting on the argparse exit code (2) rather
than pinning the literal error string — the existing `test_positive_int` unit
test in `test_cli_parser.py` only asserts `ArgumentTypeError` is raised, not
exact message text, so I matched that convention rather than introducing
message-pinning that isn't used elsewhere in this file.
---
Drafted-by: Claude Code (Sonnet 5) (no human review before posting)
--
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]
