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:
Lines 147 to 152 in 8a381cb
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
githubkit/githubkit/versions/rest.py
Lines 54 to 55 in 8a381cb
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 theresponse.raw_request.headers
notresponse.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. ๐