yanyongyu/githubkit

Underlying HTTPX clients are shared among several GitHub instances

frankie567 opened this issue ยท 7 comments

I discovered a strange behavior while testing out version v0.11.0a2. It seems that if you create several GitHub client in the same process/thread, they reuse the underlying HTTPX client, including the auth flow.

It's a problem if you need to have several clients with different kind of authentications running in your process.

Reproducible example

from githubkit import GitHub, TokenAuthStrategy, UnauthAuthStrategy


def multiple_clients() -> None:
    github_unauth = GitHub(UnauthAuthStrategy())
    response_unauth = github_unauth.rest.rate_limit.get()
    authorization_unauth = response_unauth.headers.get("authorization")
    print(authorization_unauth)  # None, as expected

    github_token = GitHub(TokenAuthStrategy("MY_TOKEN"))
    response_token = github_token.rest.rate_limit.get()
    authorization_token = response_token.headers.get("authorization")
    print(authorization_token)  # Also None, but should be `token MY TOKEN`


if __name__ == "__main__":
    multiple_clients()

Additional context

I suspect it's because ContextVar is used to retain the HTTPX client:

githubkit/githubkit/core.py

Lines 147 to 152 in 8a381cb

self.__sync_client: ContextVar[Optional[httpx.Client]] = ContextVar(
"sync_client", default=None
)
self.__async_client: ContextVar[Optional[httpx.AsyncClient]] = ContextVar(
"async_client", default=None
)


However, what I can't explain is that the same code works correctly in version 0.10.7, while ContextVar were already there. Maybe a side effect of d005552?

I can reproduce. I will look into this problem.

The problem also happens when http_cache=False, and the clients used to request api are not the same.

It seems the TokenAuthStrategy auth flow is not called.

it is caused by namespace caching in

if version in self._cached_namespaces:
return self._cached_namespaces[version]

The cache is used without considering the github instance ๐Ÿ˜‚

Oh, that's a nasty one! Glad you were able to track it down ๐Ÿ˜‰

It seems the code you provide has some small mistakes. The Authorization header is in the response.raw_request.headers not response.headers.

It seems the code you provide has some small mistakes. The Authorization header is in the response.raw_request.headers not response.headers.

Oh yeah, of course. But you got the idea ๐Ÿ˜… Glad you were able to solve it, thanks ๐Ÿ™

I have just released 0.11.0a3, with this issue fixed. ๐Ÿ˜„