On 17 Dec 2008, at 13:11, Bill Moseley wrote:
On Wed, Dec 17, 2008 at 08:34:36AM +0000, Tomas Doran wrote:
Apologies if I wasn't being clear perviously - could you convert your
suggested changes and test into a diff against the distribution which
someone could just apply with patch, rather than having to fiddle
around
copying chunks of code out of email?
Sure, but not until we can figure out what changes really need to be
made.
I think that not creating a session until something has been written
into it is a totally sane default (and providing an option to always
create one as-per the current behavior if people actually want that).
But, the plugin also always writes an "expires:" key into the store
(hence the /^session:/ match above), so if the goal is to not
write to
the store unless there's something to store then that needs looking
at, too.
Agreed.
But, if you have an application that is mostly read-only on the
session then there's the potential of timing out an active session if
it's not refreshed. Depends on the application. If using the
application naturally writes to the session often then it's not such
a problem.
I think that for backwards compatibility, we need to keep the session
updated on read.
However it would be nice to provide a way to configure the session
plugin so that reads didn't refresh the session..
And for stores that manage expiry we don't really want that extra
store call in addition to session store.
Session handling could do with refactoring as-per the authentication
plugins, so that the store and state were not plugins themselves,
this would make things a lot 'nicer'.
However, in the shorter term, providing people with a way to change
the default behaviors would go a long way.
If you'd like to include TODO tests for that in the patch (or just
fix
it, with no TODO in the test), then that would be fantastic.
As you've probably seen from the mailing list already, there are a
couple of pending patches on Catalyst-Plugin-Session already:
http://dev.catalyst.perl.org/repos/Catalyst/branches/Catalyst-Plugin-
Session/
I have not paid that much attention. What's the plan for those
branches?
The plan is to try and get some people to actually test them out
(both people who have the issues these patches solve, and people who
don't), before merging them, and then getting them released..
That's my understanding anyway, I may be wrong: the Session plugin
isn't my baby, I'm just trying to ensure the left foot knows what the
right foot is doing here. :)
Cheers
t0m
_______________________________________________
List: [email protected]
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/[email protected]/
Dev site: http://dev.catalyst.perl.org/
