-
Notifications
You must be signed in to change notification settings - Fork 949
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow client to use external Session object #1384
Conversation
I am not really clear on how the client works. Is this something that was possible in v5 but was made impossible in v6? Either way, nice fix. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is good.
Is this something we would've caught if we'd tried to use gspread for some common actions?
The client is the gspread entry point, it required some credentials and the Type of http client you want to use (standard/BackOff).
Yes it is 😑😞 that's a regression out of the box right here. In v5 it used to take the following arguments to instantiate a new client:
Now with the rework and new client it takes:
So no more ways to pass you own session object 😬
Thanks ! 🙃 |
I don't think so, this can be caught only by having a test with a custom session object. Otherwise you don't see that you miss something in your interface. |
ok I think I'm getting confused between "client" (gspread client) and "client" (http client)
Then we should also add a regression test... |
yes sorry ! that's a very good point ! we should use client for the gspread client and http_client for the actual class that handles all HTTP request (the
yes, I propose we add it later exceptionally if that's ok for you ? that would allow us to release first for the users |
Allow new `gspread.client.Client` to be initialized using an external `requests.Session` object. closes #1383 Signed-off-by: Alexandre Lavigne <[email protected]>
ca644f1
to
963097e
Compare
rebased on master branch before merge |
Allow new
gspread.client.Client
to be initialized using an externalrequests.Session
object.closes #1383