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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
