You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "ephraimbuddy (via GitHub)" <gi...@apache.org> on 2026/10/14 13:01:16 UTC
[PR] Fix Dag processor API client not catching Task SDK client errors [airflow]
ephraimbuddy opened a new pull request, #74728:
URL: https://github.com/apache/airflow/pull/74728
`DagProcessorAPIClient` (#74370) subclasses the Task SDK `Client`, which #73116 moved from `httpx` to `httpx2`. httpx2 has its own exception and transport classes, so the processor client's `except httpx.HTTPStatusError` / `httpx.HTTPError` / `httpx.ResponseNotRead` clauses stopped matching anything the client raises. Registration conflicts, closed-Job retirement and the early token-renewal fallbacks all fell through. On main, `dag_processing/test_api_client.py` and `execution_api/test_dag_processor_client.py` fail 86 of 98 tests, because `httpx.MockTransport` responses trip httpx2's `SyncByteStream` assertion.
- Move the processor client and its two test modules to `httpx2`. The test changes are the mechanical `httpx` → `httpx2` swap; no assertions changed.
- Let the Task SDK's `ServerResponseError.from_response` accept JSON errors whose `Content-Type` carries parameters (`application/json; charset=utf-8`) or different letter case. Those were raised as a bare `HTTPStatusError` with the body unread, so the processor client could not see the error `reason`.
- Ban `httpx` imports in airflow-core with ruff (`TID251`), as task-sdk already does, so the next mix-up is a lint error instead of failing tests. The six modules not yet migrated are exempt until #70522 moves them (wider migration also in #74244).
No newsfragment: the processor client has not been released yet.
Tested locally: the two processor test modules go from 86 failed / 12 passed to 98 passed, and `task-sdk/tests/task_sdk/api` passes (142), including a new test that fails without the `from_response` change. prek pre-commit (including mypy for airflow-core and task-sdk) and manual stages pass. The full suites from `breeze verify` were left to CI.
related: #70522
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes — Claude Code (Opus 5.5)
Generated-by: Claude Code (Opus 5.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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix Dag processor API client not catching Task SDK client errors [airflow]
Posted by "ephraimbuddy (via GitHub)" <gi...@apache.org>.
ephraimbuddy commented on PR #74728:
URL: https://github.com/apache/airflow/pull/74728#issuecomment-6201229530
closing as https://github.com/apache/airflow/pull/74731 was merged ahead of this
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix Dag processor API client not catching Task SDK client errors [airflow]
Posted by "ephraimbuddy (via GitHub)" <gi...@apache.org>.
ephraimbuddy closed pull request #74728: Fix Dag processor API client not catching Task SDK client errors
URL: https://github.com/apache/airflow/pull/74728
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix Dag processor API client not catching Task SDK client errors [airflow]
Posted by "kaxil (via GitHub)" <gi...@apache.org>.
kaxil commented on code in PR #74728:
URL: https://github.com/apache/airflow/pull/74728#discussion_r4284777495
##########
task-sdk/src/airflow/sdk/api/client.py:
##########
@@ -1432,7 +1432,8 @@ def from_response(cls, response: httpx2.Response) -> ServerResponseError | None:
if not (400 <= response.status_code < 600):
return None
- if response.headers.get("content-type") != "application/json":
+ media_type = response.headers.get("content-type", "").partition(";")[0].strip().lower()
Review Comment:
Which server sends `application/json; charset=utf-8` here? FastAPI's `JSONResponse` sets plain `application/json` (Starlette only appends a charset for `text/*`), so I think this branch only fires behind a proxy that rewrites the content type. That's reasonable to tolerate, but it reaches every SDK client, not just the processor: a proxy-generated JSON 404/409/410 on a heartbeat now becomes a `ServerResponseError` and takes the immediate SIGTERM path in `supervisor.py` instead of going through `_handle_heartbeat_failures`. Could the description say that, rather than framing it as a processor-client fix?
##########
airflow-core/src/airflow/dag_processing/api_client.py:
##########
@@ -28,7 +28,7 @@
from typing import Any
from uuid import UUID
-import httpx
+import httpx2
Review Comment:
This file and both test modules are already on main through #74731, which merged after this branch was last pushed. `git diff` between this head and main is empty for all three. After a rebase, what's left is the `from_response` media-type change, its test, and the ruff ban. Could you retitle around those? As it stands the squash commit would claim the #74731 fix a second time in the changelog, and the description's "fails 86 of 98 tests on main" no longer describes main.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org