Skip Menu |

This queue is for tickets about the TheSchwartz-Moosified CPAN distribution.

Report information
The Basics
Id: 71129
Status: open
Priority: 0/
Queue: TheSchwartz-Moosified

People
Owner: Nobody in particular
Requestors: revmischa [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: (no value)
Broken in: (no value)
Fixed in: (no value)



Subject: Eval without checking $@
You are committing a BRADFITZ-ism by evaling SQL statements and not checking for errors or $@ afterwards. In insert, right after for my $dbh ( $self->shuffled_databases ) { eval { $self->_try_insert($job,$dbh); }; you should warn $@ if $@. Otherwise if there are database problems (like permissions, incorrect schema search path, etc) they will be eaten up and jobs won't go in. I spent way too much time debugging why my jobs weren't being inserted.
On Wed Sep 21 15:20:48 2011, REVMISCHA wrote: Show quoted text
> You are committing a BRADFITZ-ism by evaling SQL statements and not > checking for errors or $@ afterwards. > In insert, right after > > for my $dbh ( $self->shuffled_databases ) { > eval { > $self->_try_insert($job,$dbh); > }; > > you should warn $@ if $@. Otherwise if there are database problems (like > permissions, incorrect schema search path, etc) they will be eaten up and > jobs won't go in. I spent way too much time debugging why my jobs weren't > being inserted.
The following line is: $self->debug("insert failed: $@") if $@; I believe you just need to enable debug. Now, it's arguable that a failed insert is really a debug. I argue that a failed insert is a lot more than a debug message, so in 0.07 I'm escalating it to an error (well, a warn). Thanks, -Jay
Subject: Re: [rt.cpan.org #71129] Eval without checking $@
Date: Thu, 22 Sep 2011 11:57:05 -0700
To: bug-TheSchwartz-Moosified [...] rt.cpan.org
From: Mischa Spiegelmock <mspiegelmock [...] gmail.com>
Weird, I see next unless $job->jobid; as the next line. Maybe that was an older version On Thu, Sep 22, 2011 at 11:55 AM, J. Shirley via RT <bug-TheSchwartz-Moosified@rt.cpan.org> wrote: Show quoted text
> <URL: https://rt.cpan.org/Ticket/Display.html?id=71129 > > > On Wed Sep 21 15:20:48 2011, REVMISCHA wrote:
>> You are committing a BRADFITZ-ism by evaling SQL statements and not >> checking for errors or $@ afterwards. >> In insert, right after >> >>     for my $dbh ( $self->shuffled_databases ) { >>         eval { >>             $self->_try_insert($job,$dbh); >>         }; >> >> you should warn $@ if $@. Otherwise if there are database problems (like >> permissions, incorrect schema search path, etc) they will be eaten up and >> jobs won't go in. I spent way too much time debugging why my jobs weren't >> being inserted.
> > The following line is: >            $self->debug("insert failed: $@") if $@; > > > I believe you just need to enable debug. Now, it's arguable that a > failed insert is really a debug. > > I argue that a failed insert is a lot more than a debug message, so in > 0.07 I'm escalating it to an error (well, a warn). > > Thanks, > -Jay >
On Thu Sep 22 14:57:14 2011, mspiegelmock@gmail.com wrote: Show quoted text
> Weird, I see > next unless $job->jobid; > as the next line. Maybe that was an older version > > On Thu, Sep 22, 2011 at 11:55 AM, J. Shirley via RT > <bug-TheSchwartz-Moosified@rt.cpan.org> wrote:
> > <URL: https://rt.cpan.org/Ticket/Display.html?id=71129 > > > > > On Wed Sep 21 15:20:48 2011, REVMISCHA wrote:
> >> You are committing a BRADFITZ-ism by evaling SQL statements and not > >> checking for errors or $@ afterwards. > >> In insert, right after > >> > >>     for my $dbh ( $self->shuffled_databases ) { > >>         eval { > >>             $self->_try_insert($job,$dbh); > >>         }; > >> > >> you should warn $@ if $@. Otherwise if there are database problems
(like Show quoted text
> >> permissions, incorrect schema search path, etc) they will be eaten
up and Show quoted text
> >> jobs won't go in. I spent way too much time debugging why my jobs
weren't Show quoted text
> >> being inserted.
> > > > The following line is: > >            $self->debug("insert failed: $@") if $@; > > > > > > I believe you just need to enable debug. Now, it's arguable that a > > failed insert is really a debug. > > > > I argue that a failed insert is a lot more than a debug message, so in > > 0.07 I'm escalating it to an error (well, a warn). > > > > Thanks, > > -Jay > >
With what is on CPAN now, it's there: https://metacpan.org/source/STASH/TheSchwartz-Moosified-0.06/lib/TheSchwartz/Moosified.pm#L130 I'm still migrating this to use Try::Tiny. After all, it can't be Moosified without it.