[Freeipa-devel] [PATCH] Added try/except for error handling ipautil

[Abhijeet Kasurde]

Hi All,

I have included all review comments. Please let me know your views.

On 08/17/2015 04:09 PM, Martin Basti wrote:
>
>
> On 08/17/2015 11:11 AM, Abhijeet Kasurde wrote:
>> Hi All,
>>
>> Please find the update patch with review comments,
>>
>>
>> On 08/14/2015 05:19 PM, Martin Basti wrote:
>>>
>>>
>>> On 08/14/2015 06:57 AM, Abhijeet Kasurde wrote:
>>>>
>>>> On 08/13/2015 07:08 PM, Martin Basti wrote:
>>>>>
>>>>>
>>>>> On 08/10/2015 01:47 PM, Abhijeet Kasurde wrote:
>>>>>> Hi All,
>>>>>>
>>>>>> This patch fixes bug - https://fedorahosted.org/freeipa/ticket/3406
>>>>>>
>>>>>> Thanks,
>>>>>> Abhijeet Kasurde
>>>>>>
>>>>>>
>>>>>
>>>>> Hello,
>>>>>
>>>>> thank you for the patch
>>>>>
>>>>> 1)
>>>>> -            except ValueError:
>>>>> +            except EOFError, ValueError:
>>>>>
>>>>> Please use
>>>>> except (EOFError, ValueError):
>>>>> https://docs.python.org/2/tutorial/errors.html#handling-exceptions
>>>> OK, I will include this.
>>>>> 2)
>>>>> I'm not sure if this code will work (I did not test it)
>>>>>
>>>>> I expect when stdin is closed, this will result into infinite 
>>>>> loop, because raw_input will always return EOFError.
>>>>>
>>>>>         while True:
>>>>>             try:
>>>>>                 ret = raw_input("%s: " % prompt)
>>>>>                 if allow_empty or ret.strip():
>>>>>                     return ret
>>>>>             except EOFError:
>>>>>                 pass
>>>>>
>>>> Could you please elaborate more on, so that I can include fix in 
>>>> this section of code?
>>> If you receive EOF you cannot continue in while cycle because, it 
>>> will return EOF every iteration forever.
>>>
>>> If EOF is received the while cycle must end, and appropriate action 
>>> must be take.
>>> It depends on situation, if default value is present then default 
>>> value should be used, or in case if empty value is allowed, empty 
>>> string should be returned.
>>>
>>> In case there is no default value and empty value is not allowed, 
>>> then an exception should be raised.
>>>
>>> Martin^2
>>
> NACK
>
> There is still infinity loop.
> 1)
> +            except EOFError:
> +                if allow_empty:
> +                    return ''
>
> This will continue in while cycle if allow_empty=False, you need to raise exception there.
>
> 2)
> Why so complicated? just return default looks enough to me.
> +            except EOFError:
> +                if choice.lower()[0] == "y":
> +                    return True
> +                elif choice.lower()[0] == "n":
> +                    return False
> 3)
> Remove this change please
> -
>   def get_gsserror(e):
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150817/55546c72/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-akasurde-0001-3-try-except-block-in-ipautil.patch
Type: text/x-patch
Size: 2997 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150817/55546c72/attachment.bin>