Many thanks for your great feedback, Max!<br><br><div class="gmail_quote">On Fri, Apr 16, 2010 at 12:01, Max Kanat-Alexander <span dir="ltr"><<a href="mailto:mkanat@bugzilla.org">mkanat@bugzilla.org</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"><div class="im">On 04/16/2010 01:48 AM, Yavor Nikolov wrote:<br>
> We're calling this from separate perl application - not as CGI. So I<br>
> believe error_mode is ERROR_MODE_DIE by default in our case.<br>
<br>
</div> Yeah, if you're not running in a web server, you're using<br>
ERROR_MODE_DIE by default.<br>
<div class="im"><br>
> Good to know if there are better alternatives. We have a daemon (perl<br>
> application) which is integrating version control systems (e.g.<br>
> Subversion) with Bug-tracking system (Bugzilla).<br>
<br>
</div> Yeah, I know what scmbug is. :-) I just needed to see the code of what<br>
you were doing.<br>
<div class="im"><br>
> - add comment to the bug (extracted from version control system commit<br>
> message)<br>
<br>
</div> I'd suggest doing this via the WebServices--Bug.add_comment.<br>
<div class="im"><br>
> - update bugzilla bug status and resolution; or reassign bug<br>
<br>
</div>> my @ret_list = eval {<br>
<br>
Don't run the whole block of code in an eval--just run the parts that<br>
can throw errors.<br></blockquote><div>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/).<br>
</div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"><div class="im"><br>
> my $userid = Bugzilla::User::login_to_id( $username );<br>
<br>
</div> You should probably just be doing "my $user = new Bugzilla::User({ name<br>
=> $username }) there.<br>
<br>
You could also do:<br>
<br>
my $user = eval { Bugzilla::User->check($username) };<br>
error($@) if !$user;<br>
<div class="im"><br>
> my $bug = Bugzilla::Bug->check($bugid);<br>
<br>
</div> Okay, that you can do, but wrap it in an eval and return the error to<br>
the user if there's a problem.<br>
<br>
So you'd want to do something like:<br>
<br>
my $bug = eval { Bugzilla::Bug->check($bugid) };<br>
error($@) if !$bug;<br>
<br>
Note that error() is a made-up subroutine. I just mean handle the error<br>
however scmbug normally does.<br>
<div class="im"><br>
> $bug->add_comment($comment, {isprivate => 0, work_time => 0,<br>
> type => Bugzilla::Constants->CMT_NORMAL, extra_data => ""} );<br>
<br>
</div> You don't need to specify those extra arguments if they're all at the<br>
defaults.<br>
<br>
This could *theoretically* throw an error, although it's pretty<br>
unlikely. You could put it in an eval if you *really want to*.<br>
<br>
> $bug->update();<br>
<br>
$bug->update() is not generally capable of throwing an error.<br>
<br>
> $dbh->bz_commit_transaction();<br>
<br>
You don't need bz_start_transaction or bz_commit_transaction for this<br>
block, you can get rid of them. $bug->update() will do a transaction itself.<br></blockquote><div>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).<br>
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.<br>
</div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
<div class="im"><br>
> - Nor type of error could be easily distinguished (it's just plain text).<br>
<br>
</div> You shouldn't have to distinguish the type of error, in this particular<br>
case, though--just return it to the user.<br></blockquote><div>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:<br> - 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.<br>
- 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.)<br> </div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
<div class="im"><br>
> + Recent versions of Perl support object oriented exception handling<br>
> mechanism which sounds better to me.<br>
<br>
</div> No, they don't. There are just various CPAN modules available that<br>
attempt to implement exceptions, and they probably aren't quite as good<br>
as you'd imagine.<br></blockquote><div>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: <a href="http://dev.perl.org/perl6/rfc/63.pod">http://dev.perl.org/perl6/rfc/63.pod</a>, though some improvements have been added in v 5.005 as per this article: <a href="http://www.perl.com/pub/a/2002/11/14/exception.html?page=1">http://www.perl.com/pub/a/2002/11/14/exception.html?page=1</a><br>
<br></div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
<div class="im"><br>
> I can't find the link now but I've found some discussion where Kristis<br>
> explained that WebService API was not powerful enough (maybe since the<br>
> scmbug daemon should be able to impersonate as any user without<br>
> requesting version control user to provide a password).<br>
<br>
</div> Well, okay. You could also have scmbug use one Bugzilla user for all of<br>
its changes.<br></blockquote><div>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.<br>
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 <a href="http://bz_webservice_demo.pl">bz_webservice_demo.pl</a>).<br>
</div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
<div><div></div><div class="h5"><br>
-Max<br>
--<br>
<a href="http://www.everythingsolved.com/" target="_blank">http://www.everythingsolved.com/</a><br>
Competent, Friendly Bugzilla and Perl Services. Everything Else, too.<br>
</div></div></blockquote></div><br>