Method session.get_expiry_age() does not return what the doc says
|Reported by:||martin.bouladour@…||Owned by:||nobody|
|Severity:||Normal||Keywords:||session expiration expiry age get_expiry_age|
|Has patch:||no||Needs documentation:||no|
|Needs tests:||no||Patch needs improvement:||no|
This is either a bug in the doc or in the code.
The doc for the SessionBase.get_expiry_age() method  says:
Returns the number of seconds until this session expires. For sessions with no custom expiration (or those set to expire at browser close), this will equal SESSION_COOKIE_AGE.
This means that it should return the session's remaining time to live when expiration has been customized using one of these:
A) set_expiry(seconds) # integer != 0
It works as documented in the cases B and C. But when set_expiry() has been used with seconds (A), then get_expiry_age() does the unexpected thing of returning the exact value that has been passed to set_expiry(). In other words, when you give 300 to set_expiry(), get_expiry_age() will always return 300. That is not “the number of seconds until this session expires”.
Looking at the code , it is obvious that a difference is calculated only when the expiry value is a datetime:
def get_expiry_age(self): """Get the number of seconds until the session expires.""" expiry = self.get('_session_expiry') if not expiry: # Checks both None and 0 cases return settings.SESSION_COOKIE_AGE if not isinstance(expiry, datetime): return expiry delta = expiry - timezone.now() return delta.days * 86400 + delta.seconds
I'm not sure about the purpose of this method and I don't know the design decisions that led to it, but I assume that if you don't want this function to use the session's last-modified datetime, then you may just need to edit the documentation to clarify this point (and maybe the docstring).
On the other hand, in my humble opinion, I think that this method’s behavior is quite surprising (even if it were working as documented) because the value it returns really depends on how the expiration has been defined. I believe it would be nice to make it return the number of seconds until the session expires for any active session, with no exceptional cases. And while we're at it, maybe also rename it to something that reflect better what it does, for instance "get_remaining_time_to_live" or "get_time_left". (Then again, maybe I didn't understand the purpose of this method.)
I hope this is helping.
Anyway, thank you all for this great framework!
Change History (4)
comment:1 Changed 4 years ago by aaugustin
- Needs documentation unset
- Needs tests unset
- Patch needs improvement unset
- Triage Stage changed from Unreviewed to Design decision needed
comment:2 Changed 3 years ago by aaugustin
- Triage Stage changed from Design decision needed to Accepted
comment:3 Changed 3 years ago by Aymeric Augustin <aymeric.augustin@…>
- Resolution set to fixed
- Status changed from new to closed