List:Eventum Development« Previous MessageNext Message »
From:Joao Prado Maia Date:August 26 2004 8:20pm
Subject:RE: download_emails.php cron bug (w/patch)
View as plain text  
Clay,

> The bug (in current snapshot):
> When running the download_emails.php script on a cron job,
> attempts to
> create a new issue received via e-mail result in two SQL
> errors. The example
> error handler output for those two errors is:
>

Ok, so you are referring to Issue::createFromEmail(), right?


> In Issue::addUserAssociation(), two calls are made to
> Auth::getUserID() to
> determine the ID that should be responsible for assigning
> the association.
> Since Auth::getUserID() relies on a COOKIE value, it fails
> to return a user
> ID when the script is run via cron. No user ID results in
> invalid SQL.
>

</snip>

Yes, that is a bug, but I'm not quite sure I like the fix. Instead of
creating yet another constant that would change how the code behaves,
I prefer changing Issue::addUserAssociation() to accept the user ID of
the person performing the change, and then remove the calls to
Auth::getUserID() to that ID.

This is another one of our design goals, to try to make the class
methods as self-suficient as possible. That is, it shouldn't matter if
I'm calling Issue::addUserAssociation() from the command-line or from
the Web -- it should just work by using the arguments that I pass to
it. Back when I first started developing Eventum, this wasn't obvious
so you will find from time to time a few places in the code in which
we use methods to get values external to the function, instead of
passing a parameter.


> On a related note -- after pulling my hair out tracking
> this down for much
> of the night last night, I'd really like to add proper
> Error_Handler calls
> to the error messages in all cron scripts. I sat around
> waiting for my next
> cron run a few times, only to finally give up and run the
> commands manually
> and find that the account I was testing with needed
> --fix-locks to be run.
> Lock problems on cron scripts definitely needs to be logged
> and passed off
> to the system administrator via the Eventum error e-mail
> notification.
> Otherwise, how else will know if things have ground to a halt?
>
> If you agree, I'd be happy to put that change together as well.
>

Sure, that would be nice.

--Joao

Thread
download_emails.php cron bug (w/patch)Clay Loveless26 Aug
  • RE: download_emails.php cron bug (w/patch)Joao Prado Maia26 Aug
    • Re: download_emails.php cron bug (w/patch)Clay Loveless26 Aug
      • RE: download_emails.php cron bug (w/patch)Joao Prado Maia26 Aug
        • Re: download_emails.php cron bug (w/patch)Clay Loveless27 Aug