Skip to content

SyncClient: update session with transport's headers - #203

Open
shtrom wants to merge 2 commits into
mozilla-releng:mainfrom
shtrom:issue202/rest-client-headers
Open

SyncClient: update session with transport's headers#203
shtrom wants to merge 2 commits into
mozilla-releng:mainfrom
shtrom:issue202/rest-client-headers

Conversation

@shtrom

@shtrom shtrom commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

This works around graphql-python/gql#613.

fixes: #202

@shtrom
shtrom requested a review from a team as a code owner August 25, 2026 05:21
@shtrom
shtrom force-pushed the issue202/rest-client-headers branch from bb0ded8 to 4a82554 Compare August 25, 2026 05:24
@shtrom
shtrom force-pushed the issue202/rest-client-headers branch from 4a82554 to 9f64417 Compare August 25, 2026 05:27
Comment on lines +146 to +151
# mozilla-releng/simple-github#202: work around graphql-python/gql#613.
if session.transport.headers:
session.transport.session.headers = CaseInsensitiveDict(
session.transport.headers
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This got 2 issues.

It's not in the right place. It should be in _get_gql_session or doing GQL -> REST -> GQL with the same object ends up with the second gql call behaving different.

And secondly, this overrides all headers instead of updating them. So we lose default headers (notably Accept-Encoding and User-Agent).

Something like this test shows both well:

def test_headers(responses, sync_client):
    defaults = set(requests.Session().headers)
    print("Default:", defaults)
    responses.post(GITHUB_GRAPHQL_ENDPOINT, status=200, json={"data": {"foo": "bar"}})
    responses.get(f"{GITHUB_API_ENDPOINT}/octocat", status=200, json={"answer": 42})

    sync_client.execute("query { foo }")
    before = dict(responses.calls[-1].request.headers)
    print("Before:", before)

    sync_client.get("/octocat")
    sync_client.execute("query { foo }")
    after = dict(responses.calls[-1].request.headers)
    print("After:", after)

    session = sync_client._get_requests_session()
    assert defaults <= set(session.headers)
    assert before == after
    assert session.headers["Accept"] == "application/vnd.github+json"
    assert session.headers["Authorization"] == f"Bearer {sync_client.auth._token}"

You get:

Default: {'Accept-Encoding', 'User-Agent', 'Accept', 'Connection'}
Before: {'User-Agent': 'python-requests/2.34.2', 'Accept-Encoding': 'gzip, deflate, br, zstd', 'Accept': 'application/vnd.github+json', 'Connection': 'keep-alive', 'Authorization': 'Bearer abc', 'Content-Length': '24', 'Content-Type': 'application/json'}
After: {'Accept': 'application/vnd.github+json', 'Authorization': 'Bearer abc', 'Content-Length': '24', 'Content-Type': 'application/json'}

I'm 95% sure the UA missing gets saved by urllib3 later on and that it'll work anyway (github requires a UA) but it's a very weird behavior and I don't want to have to debug something 6 months from now because the UA changes depending on request order

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.

SyncClient doesn't seem to retain authentication headers in request method

2 participants