Hi Jean,
Thanks for the review, I've redone some of these changes as you describe, could
you please take another look at them - I've updated the webrev.
Thanks,
Darren.
On 12/ 2/10 05:10 PM, jean.mccormack wrote:
> I'll review the transfer module code:
>
> cpio.py:
> line 593: destination is required here so I would put in the
> not_found_is_err=True flag . That
> means you can get rid of lines 595, 598, 599
>
> lines 602-607: ditto
>
>
> ips.py:
> lines 706-716
> I think something like this is what I envisioned
>
> origin_list = pub.get_children(Origin.ORIGIN_LABEL, Origin)
> if origin_list:
> if preferred:
> or_repo = origin.origin
> else:
> or_repo = publisher.RepositoryURI(uri=origin.origin)
> origin_name.append(or_repo)
> self.logger.debug(" Origin Info: %s", origin.origin)
>
> else:
> origin_name = self.DEF_REPO_URI
>
> line 751: put the not_found_err flag in here and remove the check at 754
>
> Jean
>
>
>
> On 12/ 2/10 09:09 AM, Darren Kenny wrote:
>> Hi,
>>
>> I'd like to request a code review for the following bug:
>>
>> 7003533 get_descendents should return an empty list when no objects
>> are found
>>
>> - http://monaco.sfbay.sun.com/detail.jsf?cr=7003533
>>
>> The webrev can be found at:
>>
>> http://cr.opensolaris.org/~dkenny/fix.7003533.slim/
>>
>> This represents a change in the API for the DataObjectBase/DataObject
>> methods:
>>
>> get_descendants()
>> get_children()
>> find_path()
>> delete_children()
>>
>> In each case, an additional parameter has been added called
>> 'not_found_is_err'
>> which is a boolean value that, if:
>>
>> - True
>>
>> Will generate an ObjectNotFoundError if an attempt to locate an object
>> which
>> doesn't exist.
>>
>> - False (Now the *Default*)
>>
>> Where there is a value returned, which is the case for all except
>> delete_children(), then if nothing is found to match the criteria, it will
>> return an empty list - [].
>>
>> This will require anyone depending on the ObjectNotFoundError to be
>> generated to
>> modify their code since the default (as requested) is to NOT fire the
>> exception
>> by default...
>>
>> I modified any code that I could find that makes use of this exception,
>> either
>> directly or just by non-action with it - so I'd like to really ensure good
>> review of this by the owners of these areas - you know who you are :)
>>
>> When doing these changes, I tried to make the logic simpler, but if there
>> were
>> cases where values weren't being tested, and as such people were relying on
>> the
>> fact an exception was being thrown, then I didn't change the code too much,
>> and
>> only added the not_found_error=True parameter, since it would be a higher
>> risk
>> when I'm unfamiliar with the code in question.
>>
>> I added tests for with/without the value being set in the install_doc, and I
>> ran
>> the existing PyUnit tests in slim to ensure there was no change in behaviour
>> in
>> other modules.
>>
>> Thanks,
>>
>> Darren.
>>
>> PS - it may be worth the CUD/DC gate applying this change and running some
>> tests in their workspace too, to see if it has any impact there...
>> _______________________________________________
>> caiman-discuss mailing list
>> [email protected]
>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
>
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
