Remove no-op ServerResponseError re-raise blocks in airflowctl operations - #70376
Conversation
…ions Every operation wrapped its request in a try/except that caught ServerResponseError only to re-raise it unchanged. The error is already raised by the client's raise_on_4xx_5xx response hook, so these wrappers added no behavior and only obscured the request logic. Dropping them leaves the propagation semantics identical while making each operation a plain request/parse.
henry3260
left a comment
henry3260
left a comment
There was a problem hiding this comment.
LGTM, I left a few suggestions to make the Dag wording consistent
|
I think there might be a couple of pre-existing endpoint bugs here (unrelated to this PR’s cleanup, but worth flagging while we’re looking):
|
Co-authored-by: Henry Chen <henryhenry0512@gmail.com>
Co-authored-by: Henry Chen <henryhenry0512@gmail.com>
Co-authored-by: Henry Chen <henryhenry0512@gmail.com>
Hmmm... indeed. but i guess it's not a good idea to change it now either 🤔 |
Backport failed to create: airflow-ctl/v0-1-test. View the failure log Run detailsNote: As of Merging PRs targeted for Airflow 3.X In matter of doubt please ask in #release-management Slack channel.
You can attempt to backport this manually by running: cherry_picker fba10d4 airflow-ctl/v0-1-testThis should apply the commit to the airflow-ctl/v0-1-test branch and leave the commit in conflict state marking After you have resolved the conflicts, you can continue the backport process by running: cherry_picker --continueIf you don't have cherry-picker installed, see the installation guide. |
|
backport in #70460 |
…ions (apache#70376) * Remove no-op ServerResponseError re-raise blocks in airflowctl operations Every operation wrapped its request in a try/except that caught ServerResponseError only to re-raise it unchanged. The error is already raised by the client's raise_on_4xx_5xx response hook, so these wrappers added no behavior and only obscured the request logic. Dropping them leaves the propagation semantics identical while making each operation a plain request/parse. * Update airflow-ctl/src/airflowctl/api/operations.py Co-authored-by: Henry Chen <henryhenry0512@gmail.com> * Update airflow-ctl/src/airflowctl/api/operations.py Co-authored-by: Henry Chen <henryhenry0512@gmail.com> * Update airflow-ctl/src/airflowctl/api/operations.py Co-authored-by: Henry Chen <henryhenry0512@gmail.com> --------- Co-authored-by: Henry Chen <henryhenry0512@gmail.com>
4 participants
Why
Every operation wrapped its request in a try/except that caught
ServerResponseErroronly to re-raise it unchanged. The error is already raised by the client's raise_on_4xx_5xx response hook, so these wrappers added no behavior and only obscured the request logic.During reviewing #69397, I found out we have lots of such usage.
What
Dropping them leaves the propagation semantics identical while making each operation a plain request/parse.
Was generative AI tooling used to co-author this PR?
Generated-by: [Claude] following the guidelines
{pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.