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

[Tomas Babej]

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.

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