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

Report information
The Basics
Id:
71129
Status:
open
Priority:
Low/Low

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

BugTracker
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.


This service runs on Request Tracker, is sponsored by The Perl Foundation, and maintained by Best Practical Solutions.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.