#22004 closed Uncategorized (wontfix)

Allow session.modified with an explicit False value to override SESSION_SAVE_EVERY_REQUEST

Reported by: bruth Owned by:
Component: contrib.sessions Version: master
Severity: Normal Keywords:
Cc: johngiannelos@… Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

This line checks precludes a view that attempts to explicitly set the session.modified attribute to False. The use case I have is have a view that serves as a "ping" endpoint for session timeouts. The problem is that if the project setting has the SESSION_SAVE_EVERY_REQUEST set to true, that view continually reset the session expiry time.

The changes required are to set this line to None and this line to if modified or (modified is None and settings.SESSION_SAVE_EVERY_REQUEST):

Change History (9)

comment:1 Changed 16 months ago by bruth

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Summary changed from Allow session.modified with an explicitl False value to override SESSION_SAVE_EVERY_REQUEST to Allow session.modified with an explicit False value to override SESSION_SAVE_EVERY_REQUEST

comment:2 Changed 16 months ago by johngian

  • Owner changed from nobody to johngian
  • Status changed from new to assigned

comment:3 Changed 16 months ago by johngian

  • Cc johngiannelos@… added

comment:4 Changed 15 months ago by anonymous

  • Owner johngian deleted
  • Status changed from assigned to new

comment:5 Changed 15 months ago by erikr

Why can't your use case be resolved by just setting SESSION_SAVE_EVERY_REQUEST to False? The issue you are describing simply sounds like this setting functioning as intended and documented.

comment:6 Changed 15 months ago by bruth

The SESSION_SAVE_EVERY_REQUEST setting is applied project-wide which prevents integrating a third-party app that needs that behavior to be turned off for it to work correctly.

comment:7 Changed 15 months ago by erikr

But can't you then document that integrating that app only works with that setting disabled? Or do users typically need this to be enabled for all their other views?

I don't think it's very intuitive for modified to have three meanings - True for always save; False for never save, and None for it to depend on SESSION_SAVE_EVERY_REQUEST. The whole point of SESSION_SAVE_EVERY_REQUEST is to always save, regardless of whether modifications have been made.

Perhaps adding a new attribute like allow_save, not necessarily with that name, would be a cleaner approach. It's definitely a lot more obvious to understand:

request.session.allow_save = False

than to realise the impact of:

request.session.modified = False

comment:8 Changed 15 months ago by bruth

I merely suggested modified = None to prevent needing to add another attribute, but I agree that a new one would be more explicit and obvious. To answer your first question, I presume the setting was introduced in the first place so that users would not have to remember to save their sessions even if nothing on the session itself changed, i.e. keep the session alive since there is a request being made. Of course the side effect is that the session would never time out as long as a request comes in.

comment:9 Changed 15 months ago by aaugustin

  • Resolution set to wontfix
  • Status changed from new to closed

Your proposal could degenerate into a war of (overrides that override)+ the default behavior :/

The root cause of the problem is that Django has global settings to control the session behavior globally. Introducing exceptions isn't a good idea.

A better idea would be to replace all relevant SESSION_* settings with a flexible mechanism to control when sessions are saved.

In the short term, may I suggest a hack: substitute request.session with a mock object in that view? Would that work?

Note: See TracTickets for help on using tickets.
Back to Top