bug 1247728 - Use sessions for requests #40
Conversation
|
This looks solid... One minor feature request is the ability to pass the session object via the constructor. I see there is no way default session... Also I see no need for resetSession... People can do my_client.session = ... I might be missing something but I see no use case for resetSession... You always want connection polling... Probably even a global one, I know I'm tempted to suggest making a single globale default session. That's what we do in node... |
|
I sort of want to make it so that someone can have a new session without having to import requests directly, but i don't feel strongly about including this method or not. I'll add the ability to pass in a session. Do we really want a global session? I feel like when I make a new API client that the object I have should be distinct from all others. |
Agree... you can accept |
| log.debug(dict((k, v) for k, v in o.items() if k != 'credentials')) | ||
|
|
||
| if session: | ||
| self.session = options.session |
There was a problem hiding this comment.
should this be: self.session = session?
There was a problem hiding this comment.
yes. that was a typo from addressing review feedback. this would still work if you pass in the session as part of options, but thats not the desired situation.
@jonasfj what do you think about this?