HttpHook and HttpAsyncHook disagree on allowed HTTP methods (surfaced by QUERY)

[regarmukesh3g]

Summary

HttpHook and HttpAsyncHook disagree about which HTTP methods they accept. The
sync hook forwards any method it is given, while the async hook rejects anything
outside a hardcoded list. The HTTP QUERY method
(draft-ietf-httpbis-safe-method-w-body)
is the case that surfaced this, but the asymmetry is broader than any one method.

I would like to check what the community thinks before opening a PR.

The asymmetry

HttpHook.run() dispatches on GET/HEAD and sends everything else through a
generic branch, so an unusual method is passed to requests untouched:

if self.method == "GET":
    req = Request(self.method, url, params=data, headers=headers, **request_kwargs)
elif self.method == "HEAD":
    req = Request(self.method, url, headers=headers, **request_kwargs)
else:
    req = Request(self.method, url, data=data, headers=headers, **request_kwargs)

method="QUERY" works today on this path — the method and the request body both
reach the wire.

HttpAsyncHook._get_request_func() instead maps method names onto per-verb
aiohttp helpers and raises on anything unlisted:

if http_method == "GET":
    return session.get
...
if http_method == "OPTIONS":
    return session.options
raise HttpMethodException(f"Unexpected HTTP Method: {http_method}")

So the same method= value that works with HttpOperator fails with
deferrable=True. Two further spots encode a similar closed set: the async
_request() attaches a body only for {"POST", "PUT", "PATCH"}, and
IDEMPOTENT_METHODS in operators/http.py lists the methods eligible for retry.

Why QUERY specifically

QUERY is a safe, idempotent method that carries a request body — intended for
searches too large or too structured for a query string. Several systems people
reach through HttpOperator (search and graph backends in particular) have begun
exposing QUERY endpoints, and the sync/async split means such a Dag silently
stops working the moment it is switched to deferrable=True.

I want to flag the standards caveat honestly: QUERY is an active IETF draft, not
part of RFC 9110, and aiohttp has no session.query() helper — a generic
session.request(method, ...) call would be needed. So this is a question about
whether Airflow wants to track a draft method, not a settled matter.

Options

1. Generalise the async dispatch. Replace the per-verb mapping with
   session.request(http_method, ...), so the async hook accepts whatever the
   sync hook accepts. Removes the asymmetry for every method at once rather than
   special-casing QUERY. Loosens a currently explicit validation, which may be
   deliberate — that is the main thing worth discussing.

2. Add QUERY to the existing lists. Smaller and more conservative: add it to
   the async dispatch, the body-carrying set, and IDEMPOTENT_METHODS. Keeps
   validation strict, but leaves the underlying divergence in place and commits
   the provider to a draft spec by name.

3. Do nothing for now, and revisit once QUERY is standardised.

I lean toward (1), because the sync/async divergence is a bug in its own right
regardless of what happens to QUERY — but I do not have the history behind the
explicit method list, so I may well be missing the reason it is written that way.

Happy to write the PR and tests for whichever direction people prefer.

---

Drafted-by: Claude Code (Opus 5); reviewed by @regarmukesh3g before posting

[artur282]

Thanks for the write-up — I read the provider source to check the asymmetry, and it is exactly as you describe. Two things I can add.

1. Option 1 is verified to work with aiohttp. I ran a local aiohttp server and client (aiohttp 3.x):

request(QUERY) -> 200 metodo recibido: QUERY

session.request(method, url, **kwargs) sends an arbitrary method token, including ones with no per-verb helper, so a generic dispatch is possible without waiting for a session.query() helper.

2. The method whitelist is not the only closed set to fix. The body/payload mapping has the same shape, in AsyncHttpSession._request() (providers/http/src/airflow/providers/http/hooks/http.py):

params=data if method == "GET" else None
data=data if method in {"POST", "PUT", "PATCH"} else None

so with method="QUERY" the body would still be dropped even after the dispatch is generalised, and DELETE with a body stays impossible. If the direction is option 1, the condition should follow what the sync branch already does (GET → params, everything else → data) rather than a hand-maintained set.

For completeness, the third spot you mention is real and already consistent with the divergence: IDEMPOTENT_METHODS = frozenset({"GET", "HEAD", "OPTIONS", "PUT", "DELETE", "TRACE"}) in providers/http/src/airflow/providers/http/operators/http.py:38, used by _warn_if_deferrable_non_idempotent_method().

On the direction: option 1 also removes the validation that currently makes a typo'd method fail loudly (method="GETT" today raises instead of hitting the network), which is a real, if small, safety property. A middle ground that keeps both: validate the token shape (method.isupper() / RFC 9110 token chars) and then dispatch generically — that way the sync and async hooks accept the same set and a malformed method is still rejected before any request is issued. IDEMPOTENT_METHODS then needs QUERY only if you want the retry/deferrable warning to treat it as safe (per the draft, it is safe and idempotent).