Uh oh!
There was an error while loading. Please reload this page.
Remove no-op exception re-raise and parser override in airflowctl - #70903
Conversation
Two pieces of airflowctl did nothing but obscure the code around them. The `except AirflowCtlNotFoundException as e: raise e` in `get_client` is the same no-op pattern cleaned out of the operations layer in apache#70376; it survived there because it sits in client.py instead. The exception is still handled by safe_call_command in cli_config.py, and api_client.close() still runs in the surviving finally. DefaultHelpParser._check_value only called super(). airflow-core's namesake does real work (check_legacy_command), but airflow-ctl deliberately has no legacy-command mapping, so this was a degenerate copy of it.
SameerMesiah97
left a comment
There was a problem hiding this comment.
Approved pending green CI.
There was a problem hiding this comment.
Pull request overview
This PR refactors airflow-ctl by removing redundant code paths that obscured control flow without providing functional value.
Changes:
- Removed a redundant
AirflowCtlNotFoundExceptioncatch-and-rethrow inget_client()while keeping thefinallycleanup. - Removed a
DefaultHelpParser._check_value()override that only delegated tosuper().
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| airflow-ctl/src/airflowctl/ctl/cli_config.py | Removes a degenerate _check_value override from the custom argparse parser. |
| airflow-ctl/src/airflowctl/api/client.py | Removes a redundant exception re-raise path in the get_client context manager and the now-unused import. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Backport successfully created: airflow-ctl/v0-1-testNote: As of Merging PRs targeted for Airflow 3.X In matter of doubt please ask in #release-management Slack channel.
|
Refactor: Remove redundant code in
airflow-ctlThis cleanup removes two pieces of
airflow-ctlcode that did nothing but obscure the surrounding logic. There is no change in behavior.1. Remove no-op exception re-raise in
client.pyget_clientfunction.except AirflowCtlNotFoundExceptionblock that only immediately re-raised the exception.try/finallyblock within a@contextlib.contextmanager. Removing theexceptclause allows the exception to propagate naturally identically to before. Thefinallyblock still executesapi_client.close(), andsafe_call_commandincli_config.pycontinues to handle the error.2. Remove redundant parser override
DefaultHelpParser._check_value.super().airflow-core's namesake does real work here (check_legacy_command),airflow-ctldeliberately has no legacy-command mapping, making this override a degenerate copy. Theerror()override in the same class actually does work and remains untouched.Note: No newsfragment is required.
airflow-ctlrelease managers regenerate the changelog directly fromgit log.Was generative AI tooling used to co-author this PR?
Generated-by: Claude Code (Opus 5) following the guidelines