SyncClient: update session with transport's headers

[shtrom]

This works around graphql-python/gql#613.

fixes: #202

[Eijebong]

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

[shtrom]

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

Yep, a dict update would be better.

It's not in the right place. It should be in _get_gql_session

I started moving this, but I have misgivings: _get_requests_session is the method that specifically reaches deep into the session to get the transport.session, which happens to not be exactly as we'd expect (which is not contractually guaranteed by gql graphql-python/gql#613 (comment)). I think it makes more sense to be fixing the session in _get_requests_session rather than in _get_gql_session (note that they are note the same “session”), though perhaps it's not desirable to morph it for everything, and instead we should copy it before updating the headers.

[shtrom]

Heh, I just found this part of a test https://github.com/shtrom/simple-github/blob/9f64417ca1f20aed1861cc58083652a07cf96ef4/test/test_client.py#L117-L120 that seems to have the same intent as the ones I added... but it doesn't check the headers of the returned session, and instead check the transport (that I suggest we update from).

Perhaps this test can be updated instead to test the session?

OTOH, I found this test when I started deepcopying the session, which broke the equality assertions here.

I'm not sure what the best approach would be here:

1. don't deepcopy.
2. remove equality assertions, and ...
3. ... potentially cache the returned session.

In favour of 1, I'm not even sure we can deepcopy the requests.Session safely.

WDYT?

[shtrom]

Hum, if we update without a deepcopy, we might as well do it ASAP, so in _get_gql_session as you suggest. But we're essentially fiddling with gql's (semi) internals.