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

[David Kupka]

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.

-- 
David Kupka
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-dkupka-0031-3-ipa-restore-Check-if-directory-is-provided-better-er.patch
Type: text/x-patch
Size: 2094 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20141121/a6f00fe4/attachment.bin>