[Fwd: Re: [scmbug-users] Error on changing status of bugs with Bugzilla 3.4.x]

[Yavor Nikolov]

Many thanks for your great feedback, Max!

On Fri, Apr 16, 2010 at 12:01, Max Kanat-Alexander <mkanat at bugzilla.org>wrote:

> On 04/16/2010 01:48 AM, Yavor Nikolov wrote:
> > We're calling this from separate perl application - not as CGI. So I
> > believe error_mode is ERROR_MODE_DIE by default in our case.
>
>         Yeah, if you're not running in a web server, you're using
> ERROR_MODE_DIE by default.
>
> > Good to know if there are better alternatives. We have a daemon (perl
> > application) which is integrating version control systems (e.g.
> > Subversion) with Bug-tracking system (Bugzilla).
>
>         Yeah, I know what scmbug is. :-) I just needed to see the code of
> what
> you were doing.
>
> >  - add comment to the bug (extracted from version control system commit
> > message)
>
>         I'd suggest doing this via the WebServices--Bug.add_comment.
>
> >  - update bugzilla bug status and resolution; or reassign bug
>
> >         my @ret_list = eval {
>
>        Don't run the whole block of code in an eval--just run the parts
> that
> can throw errors.
>
Well, maybe that's closest to what Kristis had in mind. But seems too much
bloating to me if we wrap each call to Bugzilla in eval and check each time
for errors (maybe not each function in Bugzilla is throwing errors but for
safety I'm assuming that it might throw one /some day in some release/).


>
> >         my $userid = Bugzilla::User::login_to_id( $username );
>
>         You should probably just be doing "my $user = new Bugzilla::User({
> name
> => $username }) there.
>
>        You could also do:
>
>        my $user = eval { Bugzilla::User->check($username) };
>        error($@) if !$user;
>
> >             my $bug = Bugzilla::Bug->check($bugid);
>
>         Okay, that you can do, but wrap it in an eval and return the error
> to
> the user if there's a problem.
>
>        So you'd want to do something like:
>
>        my $bug = eval { Bugzilla::Bug->check($bugid) };
>        error($@) if !$bug;
>
>        Note that error() is a made-up subroutine. I just mean handle the
> error
> however scmbug normally does.
>
> >             $bug->add_comment($comment, {isprivate => 0, work_time => 0,
> > type => Bugzilla::Constants->CMT_NORMAL, extra_data => ""} );
>
>         You don't need to specify those extra arguments if they're all at
> the
> defaults.
>
>        This could *theoretically* throw an error, although it's pretty
> unlikely. You could put it in an eval if you *really want to*.
>
> >             $bug->update();
>
>        $bug->update() is not generally capable of throwing an error.
>
> >             $dbh->bz_commit_transaction();
>
>        You don't need bz_start_transaction or bz_commit_transaction for
> this
> block, you can get rid of them. $bug->update() will do a transaction
> itself.
>
OK. I found that Object::update has start/commit transaction block but there
are some other actions in Bug::update after that which I'm not sure about
(activity log, duplicates, etc, ...)... Looking at process_bug.cgi I felt
safer to wrap this in transaction block. (I'm not sure what isolation level
is being used).
In fact - original code here called bz_lock_tables/bz_unlock_tables which
has been removed from recent Bugzilla versions: that has triggered rewriting
these parts of scmbug. As first attempt to fix it I just replaced lock with
start and unlock with commit.


>
> >  - Nor type of error could be easily distinguished (it's just plain
> text).
>
>         You shouldn't have to distinguish the type of error, in this
> particular
> case, though--just return it to the user.
>
You're right - in the particular cases we're just passing the error message
to user. My comment has been of a bit more general nature:
 - imagine we have some bulk activity - adding comments, updating status,
resolution, etc. And same error message (e.g. invalid bug id) may come out
from different places. We may want to know if it's a problem with duplicate
bug id (DUPLICATE OF); or the main bug.
 - Or we may want to distinguish between more common user errors (bad
argument provided: resolution status name, bug id, username, etc); security
error; CodeError; or a more fatal one (database, etc.)


>
> >  + Recent versions of Perl support object oriented exception handling
> > mechanism which sounds better to me.
>
>         No, they don't. There are just various CPAN modules available that
> attempt to implement exceptions, and they probably aren't quite as good
> as you'd imagine.
>
Good point (damn Perl... :-)) I got false impression that these have been
designed as part of language... But really that seems more like a wish for
future: http://dev.perl.org/perl6/rfc/63.pod, though some improvements have
been added in v 5.005 as per this article:
http://www.perl.com/pub/a/2002/11/14/exception.html?page=1


> > I can't find the link now but I've found some discussion where Kristis
> > explained that WebService API was not powerful enough (maybe since the
> > scmbug daemon should be able to impersonate as any user without
> > requesting version control user to provide a password).
>
>         Well, okay. You could also have scmbug use one Bugzilla user for
> all of
> its changes.
>
The point is that we want to see same result as if the committing user
itself has updated the bug. But maybe we could make that work if it's
possible to log-in as user with sudoer user and impersonate to someone later
- is it? At first glimpse I can't see how to call  Bugzilla::BugMail::Send(
$bugid, $vars ); either.
Anyway - definitely interaction through the WebService API is something I'd
like to experiment with (could flesh out other limitations which may be good
for WebService API improvement ideas). I hope there is a better
object-oriented approach (i.e. proxy to look like just as the real object)
to call web service methods in Perl than passing around strings (what I see
in bz_webservice_demo.pl).


>
>        -Max
> --
> http://www.everythingsolved.com/
> Competent, Friendly Bugzilla and Perl Services. Everything Else, too.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bugzilla.org/pipermail/developers/attachments/20100416/d1d1a1e7/attachment.html>