Re: [cgiapp] Re: RFC: CGI::Application::RunModeCache

[Cees Hek <suppressed>]

Hi Mark,

I have finally found a few minutes to respond to this message.  Also, 
since the mailing list has been very quiet of late (maybe everyone else 
is as busy as I am), I thought it would be a good opportunity to respond.

So my appologies for the late reply, but better late then never.

Mark Stosberg wrote:
> Cees Hek <suppressed> wrote:
>
> > Quoting Brian Cassidy <suppressed>:
> >
> > - you have effectively hijacked the prerun and postrun methods which means they
> > are unavailable to any sub/super classes.  This brings me back to my request
> > from many months ago for the ability to register prerun and postrun hooks so
> > that you can have multiple prerun/postrun execution points (patch available
> > here:
> > http://cees.crtconsulting.ca/perl/patches/CGI-Application-3.21.callbacks.patch)
> > .  As an interim solution you may want to include a comment in your docs that
> > lets people know they need to add $self->SUPER::cgiapp_postrun to their own
> > custom cgiapp_postrun method if they use it (same for cgiapp_prerun).  Otherwise
> > your cache code stops working.
>
>
> Cees,
>
> Here's some more feedback on your patch. First, it would be great if it
> was extended with some POD.

I'll throw out an excuse first.  My patch was originally presented as a 
proof of concept, and to show how easily it could be added to CGI::App, 
and in a way that did not affect any existing code.  The original 
message I sent regarding the patch can be found here:

http://www.mail-archive.com/suppressed/msg01500.html

I'll admit it is not a good excuse for not including docs, but I will 
write some if this topic is going to spark some more interest.  I would 
also be happy if others wanted to continue this idea without me (it was 
originally a group concept, not just my idea).

> To help myself and others further consider it, it would be great to see
> a couple of examples of the new syntax in use, especially if it could be
> contrasted the syntax needed without the patch.

The link I provided above has a simple example.  I can come up with 
something more concrete if required.  Also, Brian Cassidy's 
CGI::Application::RunModeCache module that he posted recently is a great 
example of a module that requires this type of feature.

The main purpose is to allow for third party modules to get access to 
the hooks available in CGI::App without making those same hooks unusably 
by the developer planning to use this third party app.  Currently, the 
hooks are a one shot deal (ie only one module can use the prerun_mode 
hook in the application).  Of course a developer can call 
$self->SUPER::prerun_mode to pass the call up the chain, but that is not 
very user friendly, and prone to mistakes.  ie a third party module 
author would have to state very clearly in the docs to 'remember to call 
$self->SUPER::prerun_mode if you decide to override the prerun_mode 
subroutine'.

> I also have some more technical feedback: 
>
> I thought it was curious that 'setup' was left out of the list of valid
> hooks.

Good point.  There probably is no good reason to leave it out.

> Also, "Positions", as implemented, don't seem as useful as they could
> me.  If I register a hook as REALLY_FIRST for teardown(), I would expect
> it would run first thing after teardown was called(). 
>
> As you've implemented it, the REALLY_FIRST teardown() position would
> happen /after/ teardown is called. 

Actually, the way I intended it was for legacy teardown() to be run in 
the MIDDLE bin.  And I think that is how it works now that I look at the 
patch again.  If register_hook is called without providing a position, 
then the MIDDLE bin is choosen by default.  And the legacy teardown() 
method is registered in the init phase with the following code:

  # Setup the default callbacks
  $self->register_hook($_ => $_) foreach VALID_HOOKS;

So, since no position is provided to register_hook, they are all 
registered in the MIDDLE bin for each hook.

> The way I think about it, most of the time one "position" would do--
> just declaring what stage you want something to run it should be good
> enough.
>
> For some special cases, I could imagine that you have something you want
> to run "really first" (or last). at a particular stage. For example,
> perhaps you want to cache a page after all other output filters have
> been applied.
>
> The trouble is, what happens when you load the next plug-in that wants
> to run "really last" at the same stage? They both can't be "really
> last".
>
> My inclination would be to initially leave "Positions" out of the patch 
> and try it out with simple stage registration. 

This idea of 'Positions' or 'Bins' was lifted directly from mod_perl. 
They use the five bins for registering handlers at the different stages 
of execution of the Apache request cycle (ie you could have multiple 
LogHandlers, or TransHandlers for each request, and this allows for a 
simple way of suggesting or requesting an order).  For the sake of 
consistency I used the exact same structure, as the two implementation 
are trying to achieve the same end (ie registering multiple functions to 
be called at different times in the execution path).  Perhaps there is a 
better (simpler) way, but I choose to stick with and existing and proven 
system.  Of course that doesn't mean that it can't be changed or 
simplified...


Hopefully it won't take me so long to respond next time :)

Cheers,

Cees

---------------------------------------------------------------------
Web Archive:  http://www.mail-archive.com/suppressed/
             http://marc.theaimsgroup.com/?l=cgiapp&r=1&w=2
To unsubscribe, e-mail: suppressed
For additional commands, e-mail: suppressed