Bug 461286
| Summary: | RFE: Bug methods should not fault when getting multiple bugs | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Community] Bugzilla | Reporter: | Will Woods <wwoods> | ||||||
| Component: | WebService | Assignee: | Noura El hawary <nelhawar> | ||||||
| Status: | CLOSED NEXTRELEASE | QA Contact: | |||||||
| Severity: | high | Docs Contact: | |||||||
| Priority: | medium | ||||||||
| Version: | devel | CC: | dkl, nelhawar | ||||||
| Target Milestone: | --- | Keywords: | FutureFeature | ||||||
| Target Release: | --- | ||||||||
| Hardware: | All | ||||||||
| OS: | All | ||||||||
| Whiteboard: | |||||||||
| Fixed In Version: | Doc Type: | Enhancement | |||||||
| Doc Text: | Story Points: | --- | |||||||
| Clone Of: | Environment: | ||||||||
| Last Closed: | 2009-02-05 05:31:02 UTC | Type: | --- | ||||||
| Regression: | --- | Mount Type: | --- | ||||||
| Documentation: | --- | CRM: | |||||||
| Verified Versions: | Category: | --- | |||||||
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||
| Cloudforms Team: | --- | Target Upstream Version: | |||||||
| Embargoed: | |||||||||
| Attachments: |
|
||||||||
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 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 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> 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 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
All done Dave and in cvs now. Thanks, Noura |
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().