[Freeipa-devel] [PATCH] 0031 ipa-restore: Check if directory is provided + better errors.

[Tomas Babej]

On 11/21/2014 02:28 PM, David Kupka wrote:
> On 11/21/2014 02:12 PM, Tomas Babej wrote:
>>
>> On 11/21/2014 01:56 PM, David Kupka wrote:
>>> On 11/21/2014 01:42 PM, Tomas Babej wrote:
>>>>
>>>> On 11/21/2014 01:33 PM, David Kupka wrote:
>>>>> https://fedorahosted.org/freeipa/ticket/4683
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> Freeipa-devel mailing list
>>>>> Freeipa-devel at redhat.com
>>>>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>>>>
>>>> -            self.read_header()
>>>> +            try:
>>>> +                self.read_header()
>>>> +            except:
>>>> +                raise admintool.ScriptError('Cannot read backup
>>>> metadata')
>>>>
>>>>
>>>> Is the bare except clause really necessary?
>>>>
>>>> https://docs.python.org/2.7/howto/doanddont.html#except
>>>>
>>>>
>>> Not really. I can't imagine other reaction to any exception raised in
>>> read_header() than complain and exit.
>>> But you're right that it can hide code errors and make debugging more
>>> complicated.
>>> Fixed patch attached.
>>>
>> On another note, I also noticed that read_header leaves leaking file
>> descriptor fd.
>>
>> Can you convert that part to use the with statement? This is a perfect
>> opportunity to fix this as you're touching related code.
>>
> I thought that python takes care of it. Thanks.
> Fixed patch attached.
>

Works quite nicely.

Thanks, ACK.

Pushed to:
master: 373bbee4e3c25fd6fb41a75b62b09d60da1a5d82
ipa-4-1: b40cf4b283c9df7d960ec80124b45d5361c75320

-- 
Tomas Babej
Associate Software Engineer | Red Hat | Identity Management
RHCE | Brno Site | IRC: tbabej | freeipa.org