Eason09053360 opened a new pull request, #72833:
URL: https://github.com/apache/airflow/pull/72833

   `airflow pools set` and `airflow pools delete` advertise `-o/--output` in 
their help, but neither command reads the value. The flag is accepted and 
silently ignored:
   
   ```
   $ airflow pools set foo 1 test -o json
   Pool foo created
   $ airflow pools delete foo -o yaml
   Pool foo deleted
   ```
   
   `pools get -o json` and `pools list -o json` honour the flag as expected; 
only `set` and `delete` do not.
   
   **Why the flag is there.** Before #12704 both commands rendered the affected 
pool as a table via `_tabulate_pools(..., tablefmt=args.output)`. That refactor 
replaced the rendering with a fixed confirmation message but left `ARG_OUTPUT` 
in the argument table. #13071 removed the same orphaned flag from `pools 
import/export` and missed `set`/`delete`, so the help has promised a format 
choice it cannot deliver ever since.
   
   **What this PR does.** Drops `ARG_OUTPUT` from the `pools set` and `pools 
delete` definitions in `cli_config.py`. The command functions are untouched; 
they never used the value. After the change the `pools` group matches 
`connections` and `variables`, where only the read commands (`list`, `get`) 
take `-o`. The CLI reference page is generated from the argument table, so it 
updates on its own.
   
   **Behaviour change to be aware of.** An invocation that passes 
`-o`/`--output` to these two commands now fails with argparse's `unrecognized 
arguments` error (exit code 2) instead of exiting 0 with the plain confirmation 
message. Invocations without the flag are unchanged. I have not added a 
newsfragment since the flag never had an effect; happy to add one if 
maintainers consider this user-facing.
   
   **Why not make the flag work instead.** The `airflowctl pools create/delete` 
successors do render the affected pool with `-o`, and the core commands are 
already marked `@deprecated_for_airflowctl` pointing at them. Restoring table 
rendering here would change the default stdout of every `pools set`/`pools 
delete` call, a larger blast radius than removing a no-op flag from a 
deprecated surface. If maintainers prefer restoring the rendering, I can do 
that instead.
   
   **Testing.** Added a parametrized parser test in `test_cli_parser.py` next 
to the other argparse-rejection tests, asserting exit code 2 and the 
`unrecognized arguments: --output json` message for both commands. With the fix 
reverted and only the test kept, exactly those two cases fail.
   
   related: #12704, #13071
   
   ---
   
   ##### Was generative AI tooling used to co-author this PR?
   
   - [X] Yes — Claude Code (Fable 5.1)
   
   Generated-by: Claude Code (Fable 5.1) following [the 
guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   


-- 
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]

Reply via email to