session callbacks (again)

3 views
Skip to first unread message

joelr

unread,
Jul 21, 2008, 2:18:27 PM7/21/08
to cherrypy-users
I have a class of applications that allocate resources at session
start and need to release those resources when the session terminates
or expires. I have followed the advice from earlier threads, in
particular, http://groups.google.com/group/cherrypy-users/browse_thread/thread/af58dc20e32d1ce1
, which suggests to subclass your favorite CP session class and
override both delete() and clean_up(). I have done this, and it works,
but it just feels... wrong.

Robert, in explaining the rationale for removing the on_delete_session
callback, you said that "it's a rare need". Really? This kind of thing
seems very common to me, but maybe I'm just thinking about it wrong??
Are there other/better approaches than trying to hook into session
start/end? As for the subclassing approach, there are two things that
bother me. First, in order to make this work (for RamSession), I had
to copy the guts of clean_up() and insert the callback into the loop
body. Yuck. Couldn't clean_up() just call delete() on expired
sessions? The other issue is that I obviously cannot change my session
storage class, e.g. to FileSession, without writing another subclass.
Callbacks just seem cleaner.

None of this is meant as criticism. I think CherryPy is terrific
(especially with ice cream!)

Cheers,
Joel


Jim Jones

unread,
Jul 21, 2008, 2:50:00 PM7/21/08
to cherryp...@googlegroups.com
On Mon, 2008-07-21 at 11:18 -0700, joelr wrote:
> Robert, in explaining the rationale for removing the on_delete_session
> callback, you said that "it's a rare need". Really? This kind of thing
> seems very common to me, but maybe I'm just thinking about it wrong??
> Are there other/better approaches than trying to hook into session
> start/end? As for the subclassing approach, there are two things that
> bother me. First, in order to make this work (for RamSession), I had
> to copy the guts of clean_up() and insert the callback into the loop
> body. Yuck. Couldn't clean_up() just call delete() on expired
> sessions? The other issue is that I obviously cannot change my session
> storage class, e.g. to FileSession, without writing another subclass.
> Callbacks just seem cleaner.
>
> None of this is meant as criticism. I think CherryPy is terrific
> (especially with ice cream!)

I agree.
There should be generic callbacks for these things so we can write
custom tools that work regardless of the actual session implementation.

Generally:
Extension by implementation subclassing is bad design, avoid it.

Extension by implementation subclassing that requires copy/paste of
critical logic from the superclass is a no-go. That's not even an
extension mechanism at all, it's monkey-patching.

I have noticed at least two areas where CP could be improved
in this regard:

1) Sessions, as Joel explained above

2) Custom dispatchers; these should really be broken down into
either a series of callbacks or some other kind of higher level
abstraction. Currently any custom dispatcher must deal intimately
with CP internals (folding the config dicts, etc.) which is the
reason why most dispatchers that you find on the wiki or elsewhere
don't work with current CP anymore.

-jj


Robert Brewer

unread,
Jul 21, 2008, 3:09:30 PM7/21/08
to cherryp...@googlegroups.com

Although I agree with the possibilities, I can't call them improvements
out of hand. The primary reason these are the way they are is to push
the complexity out to only those users who need it, rather than
complicating the distro for everyone. Adding more extension points to
the current sessions module would make it nearly as complicated as the
request process. I'm reluctant to allow an optional feature like that to
outweigh the core. Perhaps an alternative session architecture would
make that a non-issue, but I have yet to see such a proposal. I know the
"core devs" are all in favor of someone rewriting sessions from scratch,
so this is wide-open for someone to make their own.

The dispatchers are complicated because the default CP traversal
behavior is complicated. For example, file config must override
_cp_config for the same /path, but _cp_config on /path/subpath must
override file config on /path. Index and default methods also make the
CP default dispatcher more complicated than, say, a routes one. The
*requirements* of a dispatcher are pretty simple to explain: find the
right page handler and set request.config. I'd welcome alternative
implementations if they're more understandable yet enable the normal CP
behavior (and pass the test suite).


Robert Brewer
fuma...@aminus.org

Jim Jones

unread,
Jul 21, 2008, 5:06:31 PM7/21/08
to cherryp...@googlegroups.com
On Mon, 2008-07-21 at 12:09 -0700, Robert Brewer wrote:
> > I have noticed at least two areas where CP could be improved
> > in this regard:
> >
> > 1) Sessions, as Joel explained above
> >
> > 2) Custom dispatchers; these should really be broken down into
> > either a series of callbacks or some other kind of higher level
> > abstraction. Currently any custom dispatcher must deal intimately
> > with CP internals (folding the config dicts, etc.) which is the
> > reason why most dispatchers that you find on the wiki or elsewhere
> > don't work with current CP anymore.
>
> Although I agree with the possibilities, I can't call them improvements
> out of hand. The primary reason these are the way they are is to push
> the complexity out to only those users who need it, rather than
> complicating the distro for everyone. Adding more extension points to
> the current sessions module would make it nearly as complicated as the
> request process. I'm reluctant to allow an optional feature like that to
> outweigh the core. Perhaps an alternative session architecture would
> make that a non-issue, but I have yet to see such a proposal. I know the
> "core devs" are all in favor of someone rewriting sessions from scratch,
> so this is wide-open for someone to make their own.

Hmm, how would those extension points make it more
complicated for users that don't use them?

> The dispatchers are complicated because the default CP traversal
> behavior is complicated. For example, file config must override
> _cp_config for the same /path, but _cp_config on /path/subpath must
> override file config on /path. Index and default methods also make the
> CP default dispatcher more complicated than, say, a routes one. The
> *requirements* of a dispatcher are pretty simple to explain: find the
> right page handler and set request.config. I'd welcome alternative
> implementations if they're more understandable yet enable the normal CP
> behavior (and pass the test suite).

We had a short conversation about this on IRC some time ago and I still
think that an additional layer of abstraction could help to simplify the
common case.

Well, but looking at my current dispatcher I must also admit that
I'm still hard-pressed to flesh out what this layer could look like.

I can only say that being forced to populate request.config in
dispatch() feels wrong to me. Having the option to override stuff is
always welcome but this is not optional. It's a mandatory, fragile
link reaching too deep into CP's semantic belly.

I smell a bit of a contradiction in your arguments here, too; on the one
hand you're reluctant to add two straightforward hooks to sessions but
on the other you defend a fairly awkward dispatcher-hook. Fu versus
Manchu? ;-)

Anyways, neither of them is a critical flaw. I'd file both in the "rough
edges" basket, whereas the session-hooks would be a welcome fix for a
start.

-jj


Reply all
Reply to author
Forward
0 new messages