potiuk commented on PR #67860:
URL: https://github.com/apache/airflow/pull/67860#issuecomment-4596669555
Thanks for the fast turnaround on the Starlette 1.2.0 fallout — the root
cause is exactly right (the TestClient prefers `httpx2` when it's importable,
so responses become `httpx2.Response`).
One concern with pointing the tests *at* `httpx2` (`from httpx2 import
Response`, `import httpx2`, `pytest.raises(httpx2.HTTPStatusError)`): it makes
the tests **hard-depend on `httpx2` being installed**. `httpx2` is only present
transitively via `providers-common-ai → pydantic-ai-slim → genai-prices`; a
plain core sync doesn't pull it — `uv sync --project airflow-core` installs no
`httpx2`. In any environment that collects these tests without `httpx2`
present, `import httpx2` / `from httpx2 import Response` raises
`ModuleNotFoundError` at import time. It works on the CI matrix that does pull
`httpx2` (where it was failing), but it's more fragile than necessary and
couples the test to which HTTP backend happens to be installed.
`httpx2.Response` and `httpx.Response` share the same surface, and only the
status code and body are part of the API contract — so the tests don't need to
care which backend the TestClient used. Asserting on those is robust whether
`httpx`, `httpx2`, or neither is installed. Proposing to revert the `httpx2`
test edits and make the assertions backend-agnostic instead:
**`test_dag_sources.py`** — keep the import on `httpx` and drop the brittle
`isinstance` checks (the status/content/`Content-Type` assertions already cover
the behaviour):
```diff
-from httpx2 import Response
+from httpx import Response
@@
response: Response = test_client.get(f"{API_PREFIX}/{TEST_DAG_ID}",
headers={"Accept": "text/plain"})
-
- assert isinstance(response, Response)
assert response.status_code == 200
assert dag_content == response.content.decode()
@@
headers=headers,
)
- assert isinstance(response, Response)
assert response.status_code == 200
assert response.json() == {
```
**`test_xcoms.py`** — assert the status code instead of relying on the
client's exception class (drops the `httpx2`/`httpx`/`contextlib` imports):
```diff
-import contextlib
import logging
import urllib.parse
-
-import httpx
import pytest
@@
@pytest.mark.parametrize(
- ("length", "err_context"),
- [
- pytest.param(20, contextlib.nullcontext(), id="20-success"),
- pytest.param(2000, pytest.raises(httpx.HTTPStatusError),
id="2000-too-long"),
- ],
+ ("length", "expected_status"),
+ [
+ pytest.param(20, 201, id="20-success"),
+ pytest.param(2000, 400, id="2000-too-long"),
+ ],
)
- def test_xcom_set_downstream_of_mapped(self, client,
create_task_instance, session, length, err_context):
+ def test_xcom_set_downstream_of_mapped(
+ self, client, create_task_instance, session, length, expected_status
+ ):
...
- with err_context:
- response = client.post(..., params={"mapped_length": length})
- response.raise_for_status()
- task_map = session.scalars(...).one_or_none()
- assert task_map.length == length
+ response = client.post(..., params={"mapped_length": length})
+ assert response.status_code == expected_status
+ if expected_status < 400:
+ task_map = session.scalars(...).one_or_none()
+ assert task_map.length == length
```
(The `callback_runner.py` `# noqa` removal is fine as-is — no change needed
there.)
I verified this passes both **with `httpx2` installed** (the CI scenario
that was failing) and without it. Happy to push it as a fix-up to this branch
if you'd like — your call, it's your PR.
---
Drafted-by: Claude Code (Opus 4.8); reviewed by @potiuk before posting
--
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]
