GitHub user regarmukesh3g added a comment to the discussion: HttpHook and
HttpAsyncHook disagree on allowed HTTP methods (surfaced by QUERY)
Thanks — this is a better design than the one I proposed, and I reproduced both
of your findings.
**`session.request()` with an arbitrary method.** Confirmed independently
against a local aiohttp server (aiohttp 3.14.3):
```text
aiohttp generic request -> 200 method=QUERY body='q=airflow'
```
So the method token *and* the body both survive, and no `session.query()`
helper is needed.
**The body mapping is indeed a second closed set.** I had under-weighted this.
`hooks/http.py:519-520`:
```python
params=data if self.method == "GET" else None,
data=data if self.method in {"POST", "PUT", "PATCH"} else None,
```
Generalising only the dispatch would leave QUERY reaching the wire with its
body dropped — the one thing QUERY exists to carry. And your
`DELETE`-with-a-body observation is a real limitation today, independent of
whether QUERY is ever added: the sync hook sends it, the async hook cannot.
**On the middle ground:** I agree, and I'd rather have it than my option 1.
Validating the token shape and then dispatching generically keeps a property I
was about to discard — that `method="GETT"` fails immediately instead of
hitting the network. Option 1 as I first framed it traded that away without
saying so.
So the shape I'd implement, if there are no objections:
1. Validate the method token in `HttpAsyncHook`, then dispatch via
`session.request(method, ...)` instead of the per-verb mapping.
2. Make the payload mapping follow the sync branch's rule — `GET` → `params`,
everything else → `data` — rather than an enumerated set. This is what fixes
`DELETE`-with-a-body as well.
3. Leave `IDEMPOTENT_METHODS` alone in the same change. It only drives the
deferrable/retry warning, and adding `QUERY` there is a separate judgment about
tracking a draft spec — easier to review on its own than folded into a
divergence fix.
That keeps the change about the sync/async asymmetry, which is a defect on its
own terms, and leaves the "does Airflow track a draft method" question to be
settled separately.
One thing still genuinely open: no committer has weighed in, so neither of us
knows whether the explicit method list was deliberate. If it was — for example
to keep the async surface auditable — then (1) is the wrong direction and
adding `QUERY` to the three existing sets is the conservative alternative. I'd
rather hear that before writing the patch than after.
---
Drafted-by: Claude Code (Opus 5); reviewed by @regarmukesh3g before posting
GitHub link:
https://github.com/apache/airflow/discussions/72619#discussioncomment-18482879
----
This is an automatically sent email for [email protected].
To unsubscribe, please send an email to: [email protected]
