[Catalyst-dev] Re: Catalyst-dev Digest, Vol 23, Issue 2

[Evan Carroll]

On 5/6/07, catalyst-dev-request at lists.rawmode.org
<catalyst-dev-request at lists.rawmode.org> wrote:
> Send Catalyst-dev mailing list submissions to
>         catalyst-dev at lists.rawmode.org
>
> To subscribe or unsubscribe via the World Wide Web, visit
>         http://lists.rawmode.org/mailman/listinfo/catalyst-dev
> or, via email, send a message with subject or body 'help' to
>         catalyst-dev-request at lists.rawmode.org
>
> You can reach the person managing the list at
>         catalyst-dev-owner at lists.rawmode.org
>
> When replying, please edit your Subject line so it is more specific
> than "Re: Contents of Catalyst-dev digest..."
>
>
> Today's Topics:
>
>    1. Re: Bug in Catalyst::Model::DBI .15 (Evan Carroll)
>    2. Re: Bug in Catalyst::Model::DBI .15 (A. Pagaltzis)
>
>
> ----------------------------------------------------------------------
>
> Message: 1
> Date: Sun, 6 May 2007 03:48:42 -0500
> From: "Evan Carroll" <lists at evancarroll.com>
> Subject: Re: [Catalyst-dev] Bug in Catalyst::Model::DBI .15
> To: "Alex Pavlovic" <alex at taskforce-1.com>
> Cc: catalyst at lists.rawmode.org, catalyst-dev at lists.rawmode.org
> Message-ID:
>         <428b865e0705060148r1acfca45u2382e3e7288ae886 at mail.gmail.com>
> Content-Type: text/plain; charset=UTF-8; format=flowed
>
> Let's start off with a brief description of the methods of M::DBI.
> Currently, there is no connection to the database on ->new(). The
> connection is established on the first call to ->dbh, when M::DBI
> realizes it isn't connected, and proceeds to connect. This appears to
> work, though it's really just a side-effect of a method
> ->stay_connected, being called prior to an official ->connect. This
> works as long as $c->model('x') refers to the same instance, and NOT a
> rebless of that compile-time instance.
>
> The problem is with ACCEPT_CONTEXT and the current version of
> Catalyst::Model::DBI.
>
> You have to have a connection to the database, prior to having ->dbh
> called. The most common use of ACCEPT_CONEXT looks like this:
>
> sub ACCEPT_CONTEXT {
>  my ( $self, $c ) = @_;
>  bless { %$self, c => $c}, ref ( $self );
> }
>
> What this does is simple, it gives your model access to $c, and
> ensures it's unique per request.
>
> In this example, on each request, the object of an unconnected model,
> is reblessed into a new object, where it establishes connection upon
> the first call to ->dbh, unique from the compile-time copy of M::DBI.
> In other words, on each call to $c->model('DBI')->dbh, a new
> unconnected instance will have to connect, and because it's a rebless,
> its connection won't be cached ( it will neither be the same for
> another $c->model, nor will it exist outside of the one response. )
>
> So if you call this twice in your Controller:
> die $c->model('DBI')->dbh->{pg_pid}, $c->model('DBI')->dbh->{pg_pid}
> OR if you use the power this provides to call another model, that
> wraps around an M::DBI class, ex:
>
> package M::DBI::FooTable;
> sub create {
>        my $self = shift;
>        my $c = $self->{c};
>        my $fkey_bar = $c->model('DBI::BarTable')->create(@_); ## this model
> communicates on a different db session
> }
>
> You get failure.
>
> I need this functionality because I use a different model for each
> table, and on close inspection the reconnecting bit is a red flag. But
> with the current implementation, you simply can not have cross table
> transactions using the model-per-table method I've chosen.
>
> With that said, the patch provided has no drawbacks, and even if you
> only use the most minimal functionality of M::DBI, with the current
> code, you don't have a connection until the first ->dbh is called,
> which is no bueno. (work that is better handled at compile-time)
>
> The grief was simply the nature of the problem, and having to
> understand so much about the way the technology works to find the bug,
> which isn't evident in most use cases.
>
> On 5/5/07, Alex Pavlovic <alex at taskforce-1.com> wrote:
> > Hi,
> >
> > On Friday 04 May 2007 19:01, Evan Carroll wrote:
> > > M::DBI has a pretty sizable bug in as is such that it will establish a
> > > new DBI handle on every incoming connection.
> >
> > In what environment CGI, FCGI, mod_perl ? You did not mention.
> >
> > > This is a problem for
> > > speed, and transactions as it makes it much more difficult to share
> > > $dbh across other models,
> >
> > If you want to share $dbh across multiple models then take a look at:
> >
> > http://search.cpan.org/author/ALEXP/Catalyst-Model-Proxy-0.03/lib/Catalyst/Model/Proxy.pm
> >
> > It has already been done.
> >
> > > or to wrap them. I'm very surprised this
> > > went for such a long time undiscovered.
> >
> > What went on undiscovered ? I still dont understand what you are trying to
> > accomplish. You can't share dbh across threads or multiple processes( because
> > each connection results in distinct namespace ). DBIx::Class::Storage does
> > the same thing, look at:
> >
> > http://search.cpan.org/~blblack/DBIx-Class-0.07006/lib/DBIx/Class/Storage/DBI.pm
> >
> > <code>
> > elsif($self->_conn_pid != $$) {
> >           $self->_dbh->{InactiveDestroy} = 1;
> >           return $self->_dbh(undef);
> > }
> > </code>
> >
> > > This one line caused a lot of
> > > grief.
> >
> > What kind of grief ?
> >
> > >
> > > Thanks goes to >nothingmuch for all his help in first blaming the
> > > problem totally on my style, and then later totally on M::DBI.
> >
> > I assume this was on irc. Next time please contact me before posting anything,
> > it will save you some time and "grief". Not sure what your patch does here,
> > maybe you can clarify.
> >
> > >
> > > root at x60s:/usr/local/share/perl/5.8.8/Catalyst/Model# diff DBI.pm
> > > DBI_FIX.pm 59a60,62
> > >
> > > >       $self->_dbh( $self->connect );
> > >
> > > 72c75
> > > <                       } elsif ( $self->_pid != $$ ) {
> > > ---
> > >
> > > >       } elsif ( $self->_pid != $$ ) {
> > >
> > > --
> > > Evan Carroll
> > > System Lord of the Internets
> > > me at evancarroll.com
> > > 832-445-8877
> >
> > --
> > Alex Pavlovic - CTO
> > TF-1 Inc. ( Custom development, consultancy and training )
> > http://taskforce-1.com
> >
>
>
> --
> Evan Carroll
> System Lord of the Internets
> me at evancarroll.com
> 832-445-8877
>
>
>
> ------------------------------
>
> Message: 2
> Date: Sun, 6 May 2007 11:29:48 +0200
> From: "A. Pagaltzis" <pagaltzis at gmx.de>
> Subject: [Catalyst-dev] Re: Bug in Catalyst::Model::DBI .15
> To: catalyst at lists.rawmode.org, catalyst-dev at lists.rawmode.org
> Message-ID: <20070506092948.GN20785 at klangraum>
> Content-Type: text/plain; charset=utf-8
>
> * Evan Carroll <lists at evancarroll.com> [2007-05-06 10:55]:
> > With that said, the patch provided has no drawbacks, and even
> > if you only use the most minimal functionality of M::DBI, with
> > the current code, you don't have a connection until the first
> > ->dbh is called, which is no bueno. (work that is better
> > handled at compile-time)
>
> That is precisely the patch's drawback. Connecting lazily rather
> than eagerly is a feature.
>
> However, that's not hard to correct, although the patch would be
> larger: rather than squirreling away a connection in `new`,
> squirrel away a reference to an undef scalar. Then you can assign
> the handle to that scalar, and it will be shared among everyone
> who kept a copy of the same reference.
>

I don't see eye to eye with you on this one, knowing if you fail to
connect to a DB prior to pulling up a page that requires it seems like
a good functional bonus. On the flip side, I'm less concerned with
this minor detail than the larger issue of getting it fixed.

> --
> *AUTOLOAD=*_;sub _{s/(.*)::(.*)/print$2,(",$\/"," ")[defined wantarray]/e;$1}
> &Just->another->Perl->hack;
> #Aristotle
>
>
>
> ------------------------------
>
> _______________________________________________
> Catalyst-dev mailing list
> Catalyst-dev at lists.rawmode.org
> http://lists.rawmode.org/mailman/listinfo/catalyst-dev
>
>
> End of Catalyst-dev Digest, Vol 23, Issue 2
> *******************************************
>


-- 
Evan Carroll
System Lord of the Internets
me at evancarroll.com
832-445-8877