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

2015-08-19 Thread Martin Basti



On 08/19/2015 01:49 PM, Abhijeet Kasurde wrote:

Hi All,


Please find the latest patch with review comments included.

Thanks Martin for your help and review comments.

Thanks,
Abhijeet Kasurde

On 08/19/2015 05:08 PM, Martin Basti wrote:



On 08/17/2015 02:08 PM, Abhijeet Kasurde wrote:

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):






NACK

1)
Your commit message does not reflect the changes you made in code. 
The patch just solves the EOFError not Value error or various errors


2)
You removed if statement which should be there. I expected something 
like this in first case.

  while True:
-ret = raw_input(%s:  % prompt)
-if allow_empty or ret.strip():
-return ret
+try:
+ret = raw_input(%s:  % prompt)
+if allow_empty or ret.strip():
+return ret
+except EOFError:
+if allow_empty:
+   return ''
+raise RuntimeError(Failed to get user input)
Thanks
Martin




ACK

Pushed to master: 7c48621bb8e9efc47c68bb7b4af936da93325050
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

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

2015-08-19 Thread Abhijeet Kasurde

Hi All,


Please find the latest patch with review comments included.

Thanks Martin for your help and review comments.

Thanks,
Abhijeet Kasurde

On 08/19/2015 05:08 PM, Martin Basti wrote:



On 08/17/2015 02:08 PM, Abhijeet Kasurde wrote:

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):






NACK

1)
Your commit message does not reflect the changes you made in code. The 
patch just solves the EOFError not Value error or various errors


2)
You removed if statement which should be there. I expected something 
like this in first case.

  while True:
-ret = raw_input(%s:  % prompt)
-if allow_empty or ret.strip():
-return ret
+try:
+ret = raw_input(%s:  % prompt)
+if allow_empty or ret.strip():
+return ret
+except EOFError:
+if allow_empty:
+   return ''
+raise RuntimeError(Failed to get user input)
Thanks
Martin


From 1fd37e950afaa05b7e582e82bcd7dbe992f5f19d Mon Sep 17 00:00:00 2001
From: Abhijeet Kasurde akasu...@redhat.com
Date: Wed, 19 Aug 2015 17:13:43 +0530
Subject: [PATCH] Added try/except block for user_input in ipautil

Added error handling for function user_input in order to
handle EOFError in ipautil.py

https://fedorahosted.org/freeipa/ticket/3406

Signed-off-by: Abhijeet Kasurde akasu...@redhat.com
---
 ipapython/ipautil.py | 46 +-
 1 file changed, 29 insertions(+), 17 deletions(-)

diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index 88e89706b8e2aa6dea80809510d88bceaa836e85..b05a1159ab2a89fda917dd6b48afba706204579f 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -746,30 +746,40 @@ def ipa_generate_password(characters=None,pwd_len=None):
 def user_input(prompt, default = None, allow_empty = True):
 if default == None:
 while True:
-ret = raw_input(%s:  % prompt)
-if allow_empty or ret.strip():
-return ret
+try:
+ret = raw_input(%s:  % prompt)
+if allow_empty or ret.strip():
+return ret
+except EOFError:
+if allow_empty:
+return ''
+raise RuntimeError(Failed to get user input)
 
 if isinstance(default, basestring):
 while True:
-ret = raw_input(%s [%s]:  % (prompt, default))
-if not ret and (allow_empty or default):
+try:
+ret = raw_input(%s [%s]:  % (prompt, default))
+if not ret and (allow_empty or default):
+return default
+elif ret.strip():
+return ret
+   

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

2015-08-17 Thread 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):



From 380d6a0c9ac075a97da492c42169f6cf1223c1c1 Mon Sep 17 00:00:00 2001
From: Abhijeet Kasurde akasu...@redhat.com
Date: Mon, 17 Aug 2015 17:31:25 +0530
Subject: [PATCH] Added try/except block for ipautil

Added error handling for user input in order to handle various errors
like ValueError and EOFError in ipautil.

https://fedorahosted.org/freeipa/ticket/3406

Signed-off-by: Abhijeet Kasurde akasu...@redhat.com
---
 ipapython/ipautil.py | 44 +++-
 1 file changed, 27 insertions(+), 17 deletions(-)

diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index 88e89706b8e2aa6dea80809510d88bceaa836e85..c11092cb7be4e95c5ce656b87ca1e16662ad32f0 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -746,30 +746,38 @@ def ipa_generate_password(characters=None,pwd_len=None):
 def user_input(prompt, default = None, allow_empty = True):
 if default == None:
 while True:
-ret = raw_input(%s:  % prompt)
-if allow_empty or ret.strip():
-return ret
+try:
+ret = raw_input(%s:  % prompt)
+if allow_empty or ret.strip():
+return ret
+except EOFError:
+raise RuntimeError(Failed to get user input)
 
 if isinstance(default, basestring):
 while True:
-ret = raw_input(%s [%s]:  % (prompt, default))
-if not ret and (allow_empty or default):
+try:
+ret = raw_input(%s [%s]:  % (prompt, default))
+if not ret and (allow_empty or default):
+return default
+elif ret.strip():
+return ret
+except EOFError:
 return default
-elif ret.strip():
-return ret
+
 if isinstance(default, bool):
-if default:
-choice = yes
-else:
-choice = no
+choice = yes if default else no
 while True:
-ret = raw_input(%s [%s]:  % (prompt, choice))
-if not ret:
+try:
+ret = raw_input(%s [%s]:  % (prompt, choice))
+if not ret:
+return default
+elif ret.lower()[0] == y:
+return True
+elif ret.lower()[0] == n:
+return False
+except EOFError:
 return default
-elif ret.lower()[0] == y:
-return True
-elif ret.lower()[0] == n:
-return False
+
 if isinstance(default, int):
 while True:
 try:
@@ -779,6 +787,8 @@ def user_input(prompt, default = 

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

2015-08-17 Thread Abhijeet Kasurde

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


From 75dd07dcb39268232f4a49118137db6e9297d936 Mon Sep 17 00:00:00 2001
From: Abhijeet Kasurde akasu...@redhat.com
Date: Mon, 17 Aug 2015 14:36:39 +0530
Subject: [PATCH] Added try/except block for ipautil

Added error handling for user input in order to handle various errors
like ValueError and EOFError in ipautil.

https://fedorahosted.org/freeipa/ticket/3406

Signed-off-by: Abhijeet Kasurde akasu...@redhat.com
---
 ipapython/ipautil.py | 51 ---
 1 file changed, 32 insertions(+), 19 deletions(-)

diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index 88e89706b8e2aa6dea80809510d88bceaa836e85..3e3af9e95efec95eef30fcfea26dad7779b649de 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -746,30 +746,42 @@ def ipa_generate_password(characters=None,pwd_len=None):
 def user_input(prompt, default = None, allow_empty = True):
 if default == None:
 while True:
-ret = raw_input(%s:  % prompt)
-if allow_empty or ret.strip():
-return ret
+try:
+ret = raw_input(%s:  % prompt)
+if allow_empty or ret.strip():
+return ret
+except EOFError:
+if allow_empty:
+return ''
 
 if isinstance(default, basestring):
 while True:
-ret = raw_input(%s [%s]:  % (prompt, default))
-if not ret and (allow_empty or default):
+try:
+ret = raw_input(%s [%s]:  % (prompt, default))
+if not ret and (allow_empty or default):
+return default
+elif ret.strip():
+return ret
+except EOFError:
 return default
-elif ret.strip():
-return ret
+
 if isinstance(default, bool):
-if default:
-choice = yes
-else:
-choice = no
+choice = yes if default else no
 while True:
-ret = raw_input(%s [%s]:  % (prompt, choice))
-if not ret:
-return default
-elif ret.lower()[0] == y:
-return True
-elif ret.lower()[0] == n:
-return False
+try:
+ret = raw_input(%s [%s]:  % (prompt, choice))
+if not ret:
+return default
+elif ret.lower()[0] == y:
+return True
+elif ret.lower()[0] == n:
+return False
+except EOFError:
+if choice.lower()[0] == y:
+return True
+elif choice.lower()[0] == n:
+return False
+
 if isinstance(default, int):
 while True:
 try:
@@ -779,10 +791,11 @@ def user_input(prompt, default = None, allow_empty = True):
 ret = int(ret)
 except ValueError:
 pass
+except EOFError:
+return default
 else:
 return ret
 
-
 def get_gsserror(e):
 
 A GSSError exception looks differently in python 2.4 than it does
-- 
2.4.3

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

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

2015-08-17 Thread Martin Basti



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):


-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

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

2015-08-14 Thread Martin Basti



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
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

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

2015-08-13 Thread Abhijeet Kasurde


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?
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

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

2015-08-10 Thread Abhijeet Kasurde

Hi All,

This patch fixes bug - https://fedorahosted.org/freeipa/ticket/3406

Thanks,
Abhijeet Kasurde
From d1ede043e6d5d0342433e4eb769b0d48b0e4914c Mon Sep 17 00:00:00 2001
From: Abhijeet Kasurde akasu...@redhat.com
Date: Mon, 10 Aug 2015 16:56:29 +0530
Subject: [PATCH] Added try/except block for ipautil

Added error handling for user input in order to handle various errors
like ValueError and EOFError in ipautil.

https://fedorahosted.org/freeipa/ticket/3406
---
 ipapython/ipautil.py | 48 
 1 file changed, 28 insertions(+), 20 deletions(-)

diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index 88e89706b8e2aa6dea80809510d88bceaa836e85..bc6530bdc940239ce9e4f3af6021b078460c5120 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -746,30 +746,38 @@ def ipa_generate_password(characters=None,pwd_len=None):
 def user_input(prompt, default = None, allow_empty = True):
 if default == None:
 while True:
-ret = raw_input(%s:  % prompt)
-if allow_empty or ret.strip():
-return ret
+try:
+ret = raw_input(%s:  % prompt)
+if allow_empty or ret.strip():
+return ret
+except EOFError:
+pass
 
 if isinstance(default, basestring):
 while True:
-ret = raw_input(%s [%s]:  % (prompt, default))
-if not ret and (allow_empty or default):
-return default
-elif ret.strip():
-return ret
+try:
+ret = raw_input(%s [%s]:  % (prompt, default))
+if not ret and (allow_empty or default):
+return default
+elif ret.strip():
+return ret
+except EOFError:
+pass
+
 if isinstance(default, bool):
-if default:
-choice = yes
-else:
-choice = no
+choice = yes if default else no
 while True:
-ret = raw_input(%s [%s]:  % (prompt, choice))
-if not ret:
-return default
-elif ret.lower()[0] == y:
-return True
-elif ret.lower()[0] == n:
-return False
+try:
+ret = raw_input(%s [%s]:  % (prompt, choice))
+if not ret:
+return default
+elif ret.lower()[0] == y:
+return True
+elif ret.lower()[0] == n:
+return False
+except EOFError:
+pass
+
 if isinstance(default, int):
 while True:
 try:
@@ -777,7 +785,7 @@ def user_input(prompt, default = None, allow_empty = True):
 if not ret:
 return default
 ret = int(ret)
-except ValueError:
+except EOFError, ValueError:
 pass
 else:
 return ret
-- 
2.4.3

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code