-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Added Session object for cookie storage and defaults #328
Conversation
yield from asyncio.async(resp.release(), loop=loop) | ||
continue | ||
@asyncio.coroutine | ||
def options(self, url, allow_redirects=True, **kwargs): |
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.
Why allow_redirects is positional argument here when data for post/put isn't?
Question: if I'm using Cookie based auth and my cookie eventually expired, how could I renew it? Same issue with JWT tokens and other things with TTL. The solution I see is to provide a callback to session and let it call it in case of first 401 error to reauth the session. |
I think session expiration is too complex in general for implementing it into client itself. |
@asvetlov if it was so easy: I can't wrap every call with try/except/renew since I actually don't know when and where session will get expired. That's why passing some callback coroutine to trigger on auth error could be helpful. Yes, overriding .request() is what I'm doing now (: But thought that's quite common problem to have some base solution on the ground. |
break | ||
@asyncio.coroutine | ||
def head(self, url, allow_redirects=False, **kwargs): | ||
resp = yield from self.request('HEAD', url, |
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.
use hdrs.METH_XXXX instead of strings
Actually you have a need for two callbacks:
I think it's overengeneering. At least we can add those callbacks later. |
Eh, forgot about bad sites. Ok, may be I just want too much now (: Anyway, Session is a good step forward. |
i'd like to use something like base_url
|
@fafhrd91 I'll a bit extend your proposal: how about something like this? Two points:
However, all these features in additional to http-method-like class methods are a functionality of HTTP resources, not a session which should only keep and track a state, |
I'm strongly against base_url in Session class, it should be added in descendant. Adding base url is a can of worms: you have to process:
I feel adding base_url looks very appealing but think that should be implemented in another class. Processing different schema (http, https, //) and working with absolute and relative urls is not trivial. |
how often you change schema? maybe if you building proxy. if you build client for existing rest-style service it is much easier to use base_url. my feeling people use it as client for existing service more often that anything else. |
My guess is to add base_url in new class derived from Session On Tue, Apr 14, 2015 at 5:47 PM, Nikolay Kim notifications@github.com
Thanks, |
ok. let just rename to ClientSession |
ok with ClientSession name. |
i'm planing to release next 0.15 release during this week, let's include session as well. |
- Used hdrs.METH_ objects for session calls - allow_redirects now id keyword-only for get head options in session
Fixed minor issues, pointed out before. |
Added Session object for cookie storage and defaults
Simple snippet of code: