2005-06-04, Sam Tregar <sam (AT) tregar (DOT) comwrote:

A few comments:

Thanks for the feedback.
- I think add_callback() is complex enough to benefit from named
params rather than positional ones. Ex:

$self->add_callback(stage ='teardown',
sub =\&callback,
order ='FIRST');

For consistency you might have new_hook and call_hook take a
single named param as well.

Since order has been removed, that leaves us with two arguments, which I
think is reasonable to leave as positional for now.

$self->add_callback('teardown', \&callback);

We can add a named-params interface later if things change.
- I think the docs could be more explicit about exactly when each
hook is called.

I'll look at that some more.
- Ideas for additional callback hooks:

pre_init, post_init.

In general, I think we are trying get rid of positional parameters
in this initial release to keep things simple. I feel the same way about
"pre" and "post" hooks. "post_init" sounds a lot like the "prerun" stage.
It's acknowledged that there may be fighting about who runs "really
first", but adding "pre_init" just moves the first to different
location. I think that can be addressed with the current design by being
careful about which order things are loaded in and/or adding a custom
hook location.

pre_load_tmpl, post_load_tmpl, (passed the template object?),

See above for my thoughts on "pre" and "post". However, I like the idea
of having a special hook in load_tmpl() that receives the template
object.

Here's a "use case":

I have code which I think is generally useful that adds all my
configuration variables to each template, naming them like:

cfg_foo_name

Now I do this through sub-classing load_tmpl(). But that means to share
the code that way, people have to replace or create their own load_tmpl
function. With a hook there, they could simply 'use' a plugin for this,
assuming they get their config data through one of the other 'config'
plugins I would support.

on_error (called when something die()s?),

We already have error_mode() for this, and people can fuss with signal
handlers as well.

Cees, would find you a hook at error_mode() a preferable way to design
your LogDispatch plugin? Right now you advice people to roll a standard
signal handler.

Mark

2005-06-04, Cees Hek <ceeshek (AT) gmail (DOT) comwrote:

DB profiling for me has always meant turning on verbose logging in
the DB, and then running some scripts over the log files. That is
usually more work that I am willing to put in which means I dont
profile as much as I should. This definately sounds like it would
simplify things

There is a tool just for this: PQA, written in ruby.
It's easy to install and use, and runs surprisingly fast on large logs.

It was designed for PostgreSQL, but has some MySQL support too.

My power tip: Be sure to use the 'normalize' option! It's like grouping
together queries that would all use the same bind variables.

Mark

2005-06-04, Sam Tregar <sam (AT) tregar (DOT) comwrote:
Sat, 4 Jun 2005, Mark Stosberg wrote:
>
>There is a tool just for this: PQA, written in ruby.
>It's easy to install and use, and runs surprisingly fast on large logs.
>
>
>
Looks a lot like the profiling stuff that comes with DBI (which I
wrote) - DBI::ProfileDumper and dbiprof. The CGI::App I'm working on
will just be a graphical interface to those.

Sam,

Thanks for your work on the DBI profiling tool. PQA has the fundamental
feature that is not Perl-specific. For example, it revealed that 7.5% of
my query time was going to PhpPgAds, which could in theory be moved to
it's own server since it's completely seperate.

In other cases, I imagine a Perl-specific tool might be more useful,
perhaps providing line numbers in the code where a slow query was called
from.

Mark

2005-06-04, Sam Tregar <sam (AT) tregar (DOT) comwrote:
Sat, 4 Jun 2005, Michael Graham wrote:
>
>I think you're using version 4.0_2, which didn't support the class-based
>syntax. There's a draft of 4.0.3 here:
>
>
>
Right you are. Is there a good reason not to use CPAN to distribute
this release?

It's more of a logistical issue. I've got the ability to upload CGI::Application, but
Michael Graham has put together the last few proposed releases.

Mark

I've merged Michael's changes with my own tree and republished here:

It's also available through my darcs archive:

This is the preferable to use, because it remains up-to-date,
and is sometimes much easier to merge back changes.

Even if you don't use darcs for anything else, chances are there is
a binary for your platform, and it's super-easy to use. Like this:

$ darcs get
$ cd cgi-app
# hack
$ darcs record
$ darcs send

That's all it should take to generate a darcs patch and e-mail it back
to me.

I may work some on improving the documentation to make it explicit where
the callbacks are happening, and adding a hook in load_tmpl.

>on_error (called when something die()s?),
>
I like having some kind of error hook. Mucking about with $SIG{__DIE__}
can be problematic. For instance, if you use CGI::Carp and
CAP::LogDispatch, you have to be careful to chain your handlers so both
get called when something dies.

And even then, it can cause problems with other modules. For instance,
Apache::SessionX (perhaps foolishly) does some important things at
DESTRY time, and it got very confused when I set a bunch of custom
$SIG{__DIE__} handlers.

K, I don't mind having an on_error handler then. We should document
explicitly when it's in effect, though, given the following question

As an aside, is there a reason that error_mode only catches errors in
run modes, and doesn't catch errors elsewhere (e.g. in setup or prerun)?

I don't recall that being an intentional design decision of
error_mode(), but rather a historical artifact. It used to be that the
run mode was eval'ed but would then just die anyway. So the ability to
call error_mode() there was an improvement.

It does make /some sense/ to the extent that error_mode() is like a run
mode itself. If the application couldn't get "setup" to display the
target run mode, it might not be sufficiently functional to display the
error_mode() either.

If we try to take the idea to the extreme and catch errors at absolutely
any point, we have basically recreated Apache's customizable Error500
feature. ( $SIG{__DIE__} ) .

Personally, I feel like trapping just the run mode stage is a reasonable
balance between catching most the most likely errors, and being stable
enough to actually handle the error.

If people want complete control, it seems hard to avoid mucking with the
"die" signal handler anyway.

Mark