Skip to content

Merge request_kwargs headers with defaults (closes #3551)#3831

Open
genisis0x wants to merge 1 commit intoApeWorX:mainfrom
genisis0x:fix/3551-merge-request-kwargs-headers
Open

Merge request_kwargs headers with defaults (closes #3551)#3831
genisis0x wants to merge 1 commit intoApeWorX:mainfrom
genisis0x:fix/3551-merge-request-kwargs-headers

Conversation

@genisis0x
Copy link
Copy Markdown

What was wrong?

Closes #3551.

(Async)HTTPProvider.get_request_kwargs only yields the default headers from get_request_headers() when headers is absent from request_kwargs:

@to_dict
def get_request_kwargs(self) -> Iterable[tuple[str, Any]]:
    if "headers" not in self._request_kwargs:
        yield "headers", self.get_request_headers()
    yield from self._request_kwargs.items()

So passing any custom headers silently drops the required defaults:

HTTPProvider(
    endpoint_uri=URI,
    request_kwargs={"headers": {"Authorization": "Bearer ..."}},
)
# request_kwargs["headers"] -> {"Authorization": "Bearer ..."}
# Content-Type and User-Agent are gone

This matches what @souliane reported in #3551: passing request_kwargs["headers"] should extend the defaults from HTTPProvider.get_request_headers, not replace them.

How was it fixed?

get_request_kwargs now merges the defaults with the user-supplied headers (user keys win on conflict), and yields the remaining request_kwargs items unchanged.

@to_dict
def get_request_kwargs(self) -> Iterable[tuple[str, Any]]:
    user_headers = self._request_kwargs.get("headers") or {}
    yield "headers", {**self.get_request_headers(), **user_headers}
    for key, value in self._request_kwargs.items():
        if key == "headers":
            continue
        yield key, value

Same change applied to AsyncHTTPProvider.

Cute Animal Picture

cat

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Test plan

  • New unit tests in tests/core/providers/test_http_provider.py and tests/core/providers/test_async_http_provider.py covering:
    • user headers extend the defaults (Content-Type / User-Agent preserved alongside Authorization)
    • user header keys override default keys on conflict (custom User-Agent)
    • other request_kwargs (e.g. timeout) are preserved alongside merged headers
  • Existing test_get_request_headers parametrized test continues to pass (defaults unchanged when no override).
  • Newsfragment added at newsfragments/3551.bugfix.rst.

Notes

The behavior change is opt-in to anyone who was already passing request_kwargs["headers"]; previously they were dropping the defaults. After this change those callers will additionally receive the defaults, which is what the public docstring on get_request_headers already implies.

`(Async)HTTPProvider.get_request_kwargs` previously yielded the default
headers only when `headers` was absent from `request_kwargs`. Passing
`request_kwargs={"headers": {"Authorization": "Bearer ..."}}` therefore
silently dropped the required `Content-Type: application/json` and
`User-Agent` defaults.

Merge user-supplied headers on top of the defaults so callers can extend
the headers without losing the defaults; user keys still take precedence
on conflict (e.g. a custom `User-Agent` is honored).

Adds tests covering the merge, override, and unrelated kwargs cases for
both `HTTPProvider` and `AsyncHTTPProvider`.

Closes ApeWorX#3551
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update the default headers with request_kwargs passed to (Async)HTTPProvider

1 participant