Bug 461286 - RFE: Bug methods should not fault when getting multiple bugs
RFE: Bug methods should not fault when getting multiple bugs
Status: CLOSED NEXTRELEASE
Product: Bugzilla
Classification: Community
Component: WebService (Show other bugs)
devel
All All
medium Severity high (vote)
: ---
: ---
Assigned To: Noura El hawary
: FutureFeature
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-09-05 12:18 EDT by Will Woods
Modified: 2013-06-24 00:07 EDT (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Enhancement
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-02-05 00:31:02 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
v1 for adding permissive arg to Bug.get to behave politely (2.36 KB, patch)
2008-11-06 00:20 EST, Noura El hawary
dkl: review-
Details | Diff
v2 of patch to add permissive argument to Bug.get based on upstream patch (3.00 KB, patch)
2009-02-04 23:56 EST, Noura El hawary
dkl: review+
Details | Diff


External Trackers
Tracker ID Priority Status Summary Last Updated
Mozilla Foundation 458853 None None None Never

  None (edit)
Description Will Woods 2008-09-05 12:18:30 EDT
Background info: using the old RHBZ web services to fetch information about multiple bugs, we would do a MultiCall invocation of the getBug() method - one call for each bug. If one or more of the bugs was missing or unreadable, we would replace those results with an appropriate error value and return a list of results.

Bugzilla 3.x has Bug.get(), which lets us get multiple bugs. But there's a problem - if *any* of the bugs requested are missing/unreadable, we get an XML-RPC Fault and *no* return values. I could parse the Fault message, remove the bad bug ID, and retry the call - repeating until I finally get results - but that seems horribly inefficient.

Instead, I think Bug.get() should return results for each bug requested. For bugs that are missing or unreadable, a different result hash can be returned, instead of raising a fault and ending the whole call. An example error result:

{'id': bug_id, 
 'alias': alias or '', 
 'faultCode': faultCode, 
 'faultString': faultString}

A 'fault_on_error' argument could be added to Bug.get() to make it keep the current behavior, if there are things that rely on that.

This should probably apply to all the Bug methods that take multiple bug ids, and not just Bug.get().
Comment 1 Noura El hawary 2008-10-07 00:12:11 EDT
Hi Will,

The design of the WebService methods to act the way you describe was made by upstream community i have created a bug for this issue upstream, and will see what they would say, as changing it will require changing the behavior of few functions. so lets see what they think.

Noura
Comment 2 Noura El hawary 2008-11-06 00:20:06 EST
Created attachment 322669 [details]
v1 for adding permissive arg to Bug.get to behave politely

Hi Dave,

This is a patch that will add permissive argument to Bug.get so we make it behave politely as suggested by upstream, so if the user passes the permissive arg as the following:

$call = $rpc->call('Bug.get', {ids => [1,-1,-2,2], permissive => 1});

then Bug.get will return bug info for all valid bug ids and will return hash that will look like this for example for the invalid one:

                   {
                      'faultString' => '\'-1\' is not a valid bug number.',
                      'id' => '-1',
                      'faultCode' => '100'
                    },
                    {
                      'faultString' => '\'-2\' is not a valid bug number.',
                      'id' => '-2',
                      'faultCode' => '100'
                    },


and if the user sets it to 0 or doesn't pass it at all then Bug.get will behave how it is now strictly by just returning an error that an invalid bug was passed and will die. I will make another patch for the upstream review, but it will be different to this one as our Bug.get is different to theirs.

Please let me know what you think.

Noura
Comment 3 David Lawrence 2008-11-06 14:01:09 EST
Comment on attachment 322669 [details]
v1 for adding permissive arg to Bug.get to behave politely

>Index: Bugzilla/WebService/Bug.pm
>===================================================================
>-        ValidateBugID($bug_id);
>+
>+        if ($params->{permissive}) {
>+            eval { ValidateBugID($bug_id); };
>+            if ($@) {
>+                my $faultstring = $@;
>+                push(@return, {id => $bug_id, 
>+                               faultString => $faultstring->{_faultstring},
>+                               faultCode => $faultstring->{_faultcode},

You should be able to do instead:

                                 faultString => $faultstring->faultstring,
                                 faultCode => $faultstring->faultcode,
                                 
> =over
> 
>-=item C<ids>
>+=item ids

=item C<ids>

>+=item permissive
>+
>+C<boolean> An optional boolean argument, if set to 0 or ommitted then 
>+Bug.get will behave strictly by returning only an error message in the case
>+of any invalid bug ids or aliases passed to it, and if set to 1 then Bug.get
>+will return bug information for valid bug ids and faultstrings for invalid
>+bug ids.

See mkanat's comment in the upstream review about reworking the description.

>+=item faultString 
>+
>+c<string> This will only be returned for invalid bugs if the C<permissive>
>+argument was set when calling Bug.get, and it is an error indicating that 
>+the bug id was invalid.

C<string>

>+=item faultCode
>+
>+c<int> This will only be returned for invalid bugs if the C<permissive>
>+argument was set when calling Bug.get, and it is the error code for the 
>+invalid bug error.

C<int>
Comment 4 Noura El hawary 2009-02-04 23:56:12 EST
Created attachment 330958 [details]
v2 of patch to add permissive argument to Bug.get based on upstream patch

Hey Dave, here is a patch that is based on the upstream patch with little changes to match out code version.

Noura
Comment 5 David Lawrence 2009-02-05 00:09:10 EST
Comment on attachment 330958 [details]
v2 of patch to add permissive argument to Bug.get based on upstream patch

Looks good to me Noura and works as expected. Make sure to add the doc changes mentioned by Max and then feel free to checkin.

Dave
Comment 6 Noura El hawary 2009-02-05 00:31:02 EST
All done Dave and in cvs now.

Thanks,
Noura

Note You need to log in before you can comment on or make changes to this bug.