[PATCH] issue #330: Properly log the error message when login fails
From: Aline Manera <alinefm@br.ibm.com> The error message and all its parameters must be string. So convert error code to string in order to accomplish it. Signed-off-by: Aline Manera <alinefm@br.ibm.com> --- src/kimchi/auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/kimchi/auth.py b/src/kimchi/auth.py index af3b610..b16f2db 100644 --- a/src/kimchi/auth.py +++ b/src/kimchi/auth.py @@ -107,7 +107,7 @@ def authenticate(username, password, service="passwd"): try: auth.authenticate() except PAM.error, (resp, code): - msg_args = {'userid': username, 'code': code} + msg_args = {'userid': username, 'code': str(code)} raise OperationFailed("KCHAUTH0001E", msg_args) return True -- 1.7.10.4
Reviewed-by: Daniel Barboza <danielhb@linux.vnet.ibm.com> On 03/04/2014 04:35 PM, Aline Manera wrote:
From: Aline Manera <alinefm@br.ibm.com>
The error message and all its parameters must be string. So convert error code to string in order to accomplish it.
Signed-off-by: Aline Manera <alinefm@br.ibm.com> --- src/kimchi/auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/kimchi/auth.py b/src/kimchi/auth.py index af3b610..b16f2db 100644 --- a/src/kimchi/auth.py +++ b/src/kimchi/auth.py @@ -107,7 +107,7 @@ def authenticate(username, password, service="passwd"): try: auth.authenticate() except PAM.error, (resp, code): - msg_args = {'userid': username, 'code': code} + msg_args = {'userid': username, 'code': str(code)} raise OperationFailed("KCHAUTH0001E", msg_args)
return True
2014/3/5 3:35, Aline Manera:
From: Aline Manera <alinefm@br.ibm.com>
The error message and all its parameters must be string. So convert error code to string in order to accomplish it.
I think the code is str already. Why should it be converted explicitly? Is there any error encountered?
Signed-off-by: Aline Manera <alinefm@br.ibm.com> --- src/kimchi/auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/kimchi/auth.py b/src/kimchi/auth.py index af3b610..b16f2db 100644 --- a/src/kimchi/auth.py +++ b/src/kimchi/auth.py @@ -107,7 +107,7 @@ def authenticate(username, password, service="passwd"): try: auth.authenticate() except PAM.error, (resp, code): - msg_args = {'userid': username, 'code': code} + msg_args = {'userid': username, 'code': str(code)} raise OperationFailed("KCHAUTH0001E", msg_args)
return True
On 03/05/2014 09:49 AM, Shu Ming wrote:
2014/3/5 3:35, Aline Manera:
From: Aline Manera <alinefm@br.ibm.com>
The error message and all its parameters must be string. So convert error code to string in order to accomplish it.
I think the code is str already. Why should it be converted explicitly? Is there any error encountered? agree. Also python can converted the parameters well when format the string. This is different with C code.
such as: In [218]: "This is a code test: %(code)s " % {"code": 2} Out[218]: 'This is a code test: 2 ' so is this is need? IMO, if we still need it to be converted explicitly. We can use %d, instead of %s as follow. In [221]: "This is a code test: %(code)d " % {"code": 2} Out[221]: 'This is a code test: 2 '
Signed-off-by: Aline Manera <alinefm@br.ibm.com> --- src/kimchi/auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/kimchi/auth.py b/src/kimchi/auth.py index af3b610..b16f2db 100644 --- a/src/kimchi/auth.py +++ b/src/kimchi/auth.py @@ -107,7 +107,7 @@ def authenticate(username, password, service="passwd"): try: auth.authenticate() except PAM.error, (resp, code): - msg_args = {'userid': username, 'code': code} + msg_args = {'userid': username, 'code': str(code)} raise OperationFailed("KCHAUTH0001E", msg_args)
return True
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
-- Thanks and best regards! Sheldon Feng(冯少合)<shaohef@linux.vnet.ibm.com> IBM Linux Technology Center
On 03/05/2014 11:59 AM, Sheldon wrote:
On 03/05/2014 09:49 AM, Shu Ming wrote:
2014/3/5 3:35, Aline Manera:
From: Aline Manera <alinefm@br.ibm.com>
The error message and all its parameters must be string. So convert error code to string in order to accomplish it.
I think the code is str already. Why should it be converted explicitly? Is there any error encountered? agree. Also python can converted the parameters well when format the string. This is different with C code.
such as: In [218]: "This is a code test: %(code)s " % {"code": 2} Out[218]: 'This is a code test: 2 '
so is this is need? IMO, if we still need it to be converted explicitly. We can use %d, instead of %s as follow.
In [221]: "This is a code test: %(code)d " % {"code": 2} Out[221]: 'This is a code test: 2 '
for int, it is no need to be converted explicitly In [250]: int.__str__ Type: wrapper_descriptor String Form:<slot wrapper '__str__' of 'int' objects> Namespace: Python builtin Docstring: x.__str__() <==> str(x) How I will give a example about how %s works. In [234]: class a(object): .....: def __str__(self): .....: return "b" .....: In [235]: class ca(object): def __str__(self): return "b" .....: In [236]: class ca(object): .....: def __str__(self): .....: return "b" .....: In [237]: ia = ca() In [238]: print ia b In [239]: "%s" % ia Out[239]: 'b' In [240]: class ca(object): pass In [241]: ia = ca() In [242]: print ia <__main__.ca object at 0x2b705d0> In [243]: "%s" % ia Out[243]: '<__main__.ca object at 0x2b705d0>'
Signed-off-by: Aline Manera <alinefm@br.ibm.com> --- src/kimchi/auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/kimchi/auth.py b/src/kimchi/auth.py index af3b610..b16f2db 100644 --- a/src/kimchi/auth.py +++ b/src/kimchi/auth.py @@ -107,7 +107,7 @@ def authenticate(username, password, service="passwd"): try: auth.authenticate() except PAM.error, (resp, code): - msg_args = {'userid': username, 'code': code} + msg_args = {'userid': username, 'code': str(code)} raise OperationFailed("KCHAUTH0001E", msg_args)
return True
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
-- Thanks and best regards! Sheldon Feng(冯少合)<shaohef@linux.vnet.ibm.com> IBM Linux Technology Center
On 03/04/2014 10:49 PM, Shu Ming wrote:
2014/3/5 3:35, Aline Manera:
From: Aline Manera <alinefm@br.ibm.com>
The error message and all its parameters must be string. So convert error code to string in order to accomplish it.
I think the code is str already. Why should it be converted explicitly? Is there any error encountered?
Yes. Take a look in github: https://github.com/kimchi-project/kimchi/issues/330 The error happens because KimchiException() converts all message and arguments to unicode So we need to ensure the arguments are strings or do it convert it on KimchiException() which I think can cause problems when a different type which does not have str representattion
Signed-off-by: Aline Manera <alinefm@br.ibm.com> --- src/kimchi/auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/kimchi/auth.py b/src/kimchi/auth.py index af3b610..b16f2db 100644 --- a/src/kimchi/auth.py +++ b/src/kimchi/auth.py @@ -107,7 +107,7 @@ def authenticate(username, password, service="passwd"): try: auth.authenticate() except PAM.error, (resp, code): - msg_args = {'userid': username, 'code': code} + msg_args = {'userid': username, 'code': str(code)} raise OperationFailed("KCHAUTH0001E", msg_args)
return True
On 03/06/2014 01:31 AM, Aline Manera wrote:
On 03/04/2014 10:49 PM, Shu Ming wrote:
2014/3/5 3:35, Aline Manera:
From: Aline Manera <alinefm@br.ibm.com>
The error message and all its parameters must be string. So convert error code to string in order to accomplish it.
I think the code is str already. Why should it be converted explicitly? Is there any error encountered?
Yes. Take a look in github: https://github.com/kimchi-project/kimchi/issues/330
The error happens because KimchiException() converts all message and arguments to unicode So we need to ensure the arguments are strings or do it convert it on KimchiException() which I think can cause problems when a different type which does not have str representattion
I have check the code is really a int. Now I check the follow exception.py: for key, value in args.iteritems(): if not isinstance(value, unicode): args[key] = unicode(value, 'utf-8') return unicode(translation.gettext(text), 'utf-8') % args it is as you said, unicode require the arguments are strings. but I have a try the follow code can work, with unicode(value, 'utf-8'). In [22]: unicode(translation.gettext("你好 %(code)s"), 'utf-8') % {"code": 1} Out[22]: u'\u4f60\u597d 1' or In [31]: translation.gettext(u"你好 %(code)s") % {"code": 1} Out[31]: u'\u4f60\u597d 1' I wonder can we improve the KimchiException()? or we need to convert every where when we call KimchiException str(int) is looks like strange for a python developer.
Signed-off-by: Aline Manera <alinefm@br.ibm.com> --- src/kimchi/auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/kimchi/auth.py b/src/kimchi/auth.py index af3b610..b16f2db 100644 --- a/src/kimchi/auth.py +++ b/src/kimchi/auth.py @@ -107,7 +107,7 @@ def authenticate(username, password, service="passwd"): try: auth.authenticate() except PAM.error, (resp, code): - msg_args = {'userid': username, 'code': code} + msg_args = {'userid': username, 'code': str(code)} raise OperationFailed("KCHAUTH0001E", msg_args)
return True
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
-- Thanks and best regards! Sheldon Feng(冯少合)<shaohef@linux.vnet.ibm.com> IBM Linux Technology Center
On 03/06/2014 01:31 AM, Aline Manera wrote:
On 03/04/2014 10:49 PM, Shu Ming wrote:
2014/3/5 3:35, Aline Manera:
From: Aline Manera <alinefm@br.ibm.com>
The error message and all its parameters must be string. So convert error code to string in order to accomplish it.
I think the code is str already. Why should it be converted explicitly? Is there any error encountered?
Yes. Take a look in github: https://github.com/kimchi-project/kimchi/issues/330
The error happens because KimchiException() converts all message and arguments to unicode So we need to ensure the arguments are strings or do it convert it on KimchiException() which I think can cause problems when a different type which does not have str representattion
Both the python build in objects and the objects derived from python object class has string representation. Only the object derived from nothing has not. In this case get string representation will raise Exception. We can see this is a bug.
Signed-off-by: Aline Manera <alinefm@br.ibm.com> --- src/kimchi/auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/kimchi/auth.py b/src/kimchi/auth.py index af3b610..b16f2db 100644 --- a/src/kimchi/auth.py +++ b/src/kimchi/auth.py @@ -107,7 +107,7 @@ def authenticate(username, password, service="passwd"): try: auth.authenticate() except PAM.error, (resp, code): - msg_args = {'userid': username, 'code': code} + msg_args = {'userid': username, 'code': str(code)} raise OperationFailed("KCHAUTH0001E", msg_args)
return True
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
-- Thanks and best regards! Sheldon Feng(冯少合)<shaohef@linux.vnet.ibm.com> IBM Linux Technology Center
On 03/05/2014 03:35 AM, Aline Manera wrote:
From: Aline Manera <alinefm@br.ibm.com>
The error message and all its parameters must be string. So convert error code to string in order to accomplish it.
Signed-off-by: Aline Manera <alinefm@br.ibm.com> --- src/kimchi/auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/kimchi/auth.py b/src/kimchi/auth.py index af3b610..b16f2db 100644 --- a/src/kimchi/auth.py +++ b/src/kimchi/auth.py @@ -107,7 +107,7 @@ def authenticate(username, password, service="passwd"): try: auth.authenticate() except PAM.error, (resp, code): - msg_args = {'userid': username, 'code': code} + msg_args = {'userid': username, 'code': str(code)} raise OperationFailed("KCHAUTH0001E", msg_args)
return True How about the follow change? It can avoid call str() before we raise KimchiException Everywhere. Also I think it is not good to other developers to call str explicitly Everywhere.
diff --git a/src/kimchi/exception.py b/src/kimchi/exception.py index 71a4d11..73e88ce 100644 --- a/src/kimchi/exception.py +++ b/src/kimchi/exception.py @@ -50,7 +50,7 @@ class KimchiException(Exception): translation = gettext for key, value in args.iteritems(): - if not isinstance(value, unicode): + if isinstance(value, str): args[key] = unicode(value, 'utf-8') return unicode(translation.gettext(text), 'utf-8') % args -- Thanks and best regards! Sheldon Feng(冯少合)<shaohef@linux.vnet.ibm.com> IBM Linux Technology Center
On 03/06/2014 06:41 AM, Sheldon wrote:
On 03/05/2014 03:35 AM, Aline Manera wrote:
From: Aline Manera <alinefm@br.ibm.com>
The error message and all its parameters must be string. So convert error code to string in order to accomplish it.
Signed-off-by: Aline Manera <alinefm@br.ibm.com> --- src/kimchi/auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/kimchi/auth.py b/src/kimchi/auth.py index af3b610..b16f2db 100644 --- a/src/kimchi/auth.py +++ b/src/kimchi/auth.py @@ -107,7 +107,7 @@ def authenticate(username, password, service="passwd"): try: auth.authenticate() except PAM.error, (resp, code): - msg_args = {'userid': username, 'code': code} + msg_args = {'userid': username, 'code': str(code)} raise OperationFailed("KCHAUTH0001E", msg_args)
return True How about the follow change? It can avoid call str() before we raise KimchiException Everywhere. Also I think it is not good to other developers to call str explicitly Everywhere.
diff --git a/src/kimchi/exception.py b/src/kimchi/exception.py index 71a4d11..73e88ce 100644 --- a/src/kimchi/exception.py +++ b/src/kimchi/exception.py @@ -50,7 +50,7 @@ class KimchiException(Exception): translation = gettext
for key, value in args.iteritems(): - if not isinstance(value, unicode): + if isinstance(value, str): args[key] = unicode(value, 'utf-8')
return unicode(translation.gettext(text), 'utf-8') % args
What about always convert to string before to unicode? if not isinstance(value, unicode): args[key] = unicode(*str*(value), 'utf-8') That way we guarantee everything will be unicode in the end.
Am 06-03-2014 11:53, schrieb Aline Manera:
What about always convert to string before to unicode?
if not isinstance(value, unicode): args[key] = unicode(*str*(value), 'utf-8')
That way we guarantee everything will be unicode in the end. I agree. Regardless of the the parameter type, everything can be converted to string (and subsequently to unicode).
On 03/06/2014 10:53 PM, Aline Manera wrote:
On 03/06/2014 06:41 AM, Sheldon wrote:
On 03/05/2014 03:35 AM, Aline Manera wrote:
From: Aline Manera <alinefm@br.ibm.com>
The error message and all its parameters must be string. So convert error code to string in order to accomplish it.
Signed-off-by: Aline Manera <alinefm@br.ibm.com> --- src/kimchi/auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/kimchi/auth.py b/src/kimchi/auth.py index af3b610..b16f2db 100644 --- a/src/kimchi/auth.py +++ b/src/kimchi/auth.py @@ -107,7 +107,7 @@ def authenticate(username, password, service="passwd"): try: auth.authenticate() except PAM.error, (resp, code): - msg_args = {'userid': username, 'code': code} + msg_args = {'userid': username, 'code': str(code)} raise OperationFailed("KCHAUTH0001E", msg_args)
return True How about the follow change? It can avoid call str() before we raise KimchiException Everywhere. Also I think it is not good to other developers to call str explicitly Everywhere.
diff --git a/src/kimchi/exception.py b/src/kimchi/exception.py index 71a4d11..73e88ce 100644 --- a/src/kimchi/exception.py +++ b/src/kimchi/exception.py @@ -50,7 +50,7 @@ class KimchiException(Exception): translation = gettext
for key, value in args.iteritems(): - if not isinstance(value, unicode): + if isinstance(value, str): args[key] = unicode(value, 'utf-8')
return unicode(translation.gettext(text), 'utf-8') % args
What about always convert to string before to unicode?
if not isinstance(value, unicode): args[key] = unicode(*str*(value), 'utf-8') This is also OK. seems it is more understand to most python developer. actually it is same effect with the above code I suggested.
AFAK， seems two types has no string representation, one is unicode and another is object derived from nothing. The later object derived from nothing, we can consider it is a bug. Kimchi do not allowed this. class c1(): pass But I'm not worry about it. IMO, no one will not pass this instance of object to KimchiException as args. we just allow objects derived from python object class in kimchi, class c2(object): pass
That way we guarantee everything will be unicode in the end.
-- Thanks and best regards! Sheldon Feng(冯少合)<shaohef@linux.vnet.ibm.com> IBM Linux Technology Center
On 03/06/2014 11:12 PM, Sheldon wrote:
On 03/06/2014 10:53 PM, Aline Manera wrote:
On 03/06/2014 06:41 AM, Sheldon wrote:
On 03/05/2014 03:35 AM, Aline Manera wrote:
From: Aline Manera <alinefm@br.ibm.com>
The error message and all its parameters must be string. So convert error code to string in order to accomplish it.
Signed-off-by: Aline Manera <alinefm@br.ibm.com> --- src/kimchi/auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/kimchi/auth.py b/src/kimchi/auth.py index af3b610..b16f2db 100644 --- a/src/kimchi/auth.py +++ b/src/kimchi/auth.py @@ -107,7 +107,7 @@ def authenticate(username, password, service="passwd"): try: auth.authenticate() except PAM.error, (resp, code): - msg_args = {'userid': username, 'code': code} + msg_args = {'userid': username, 'code': str(code)} raise OperationFailed("KCHAUTH0001E", msg_args)
return True How about the follow change? It can avoid call str() before we raise KimchiException Everywhere. Also I think it is not good to other developers to call str explicitly Everywhere.
diff --git a/src/kimchi/exception.py b/src/kimchi/exception.py index 71a4d11..73e88ce 100644 --- a/src/kimchi/exception.py +++ b/src/kimchi/exception.py @@ -50,7 +50,7 @@ class KimchiException(Exception): translation = gettext
for key, value in args.iteritems(): - if not isinstance(value, unicode): + if isinstance(value, str): args[key] = unicode(value, 'utf-8')
return unicode(translation.gettext(text), 'utf-8') % args
What about always convert to string before to unicode?
if not isinstance(value, unicode): args[key] = unicode(*str*(value), 'utf-8') This is also OK. seems it is more understand to most python developer. actually it is same effect with the above code I suggested.
AFAK, seems two types has no string representation, one is unicode and another is object derived from nothing. sorry, unicode has string representation. but it make no sense. For it will raise UnicodeEncodeError.
The later object derived from nothing, we can consider it is a bug. Kimchi do not allowed this. class c1(): pass But I'm not worry about it. IMO, no one will not pass this instance of object to KimchiException as args.
we just allow objects derived from python object class in kimchi, class c2(object): pass
That way we guarantee everything will be unicode in the end.
-- Thanks and best regards!
Sheldon Feng(???)<shaohef@linux.vnet.ibm.com> IBM Linux Technology Center
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
-- Thanks and best regards! Sheldon Feng(???)<shaohef@linux.vnet.ibm.com> IBM Linux Technology Center
Am 06-03-2014 12:12, schrieb Sheldon:
AFAK, seems two types has no string representation, one is unicode and another is object derived from nothing. The statement if not isinstance(value, unicode), which is in the sample code above, makes sure that we will not try to convert a unicode object to string (only those who have a different type).
class X(): ... def x(self): ... pass ... a = X() print str(a) <__main__.X instance at 0x7f8fe13d4b00> print "this is a string: %s." % a
And an object derived from nothing also has a string representation. Take a look at this example: this is a string: <__main__.X instance at 0x7f8fe13d4b00>.
But I'm not worry about it. IMO, no one will not pass this instance of object to KimchiException as args. IMO, we should never trust that the users/developers will pass the correct parameters to our code. Eventually, someone will forget that, and then we will have one more bug ;)
On 03/06/2014 11:46 PM, Crístian Viana wrote:
Am 06-03-2014 12:12, schrieb Sheldon:
AFAK, seems two types has no string representation, one is unicode and another is object derived from nothing. The statement if not isinstance(value, unicode), which is in the sample code above, makes sure that we will not try to convert a unicode object to string (only those who have a different type).
And an object derived from nothing also has a string representation. Take a look at this example:
class X(): ... def x(self): ... pass ... a = X() print str(a) <__main__.X instance at 0x7f8fe13d4b00> print "this is a string: %s." % a this is a string: <__main__.X instance at 0x7f8fe13d4b00>.
You forgot one. The a follow string format will also do string representation. In [2]: class a(): pass In [9]: def toU1(value): ...: if not isinstance(value, unicode): ...: value = unicode(str(value), 'utf-8') ...: return u"fo(o( = %s" % value ...: In [10]: def toU2(value): ....: if isinstance(value, str): ....: value = unicode(str(value), 'utf-8') ....: return u"fo(o( = %s" % value ....: In [11]: print toU1(a) fo(o( = __main__.a In [12]: print toU2(a) fo(o( = __main__.a In [13]: print toU1("fo(o(") fo(o( = fo(o( In [14]: print toU2("fo(o(") fo(o( = fo(o( In [15]: print toU1(u"fo(o(") fo(o( = fo(o( In [16]: print toU2(u"fo(o(") fo(o( = fo(o( if one developer define a class like this, it should tell the ptyhon this a unicode file. In [17]: class afo(o((): pass We have discuss this problem before, kimchi do not like this. In this case, ptyhon take a string are unicode, it should OK. Not try.
But I'm not worry about it. IMO, no one will not pass this instance of object to KimchiException as args. IMO, we should never trust that the users/developers will pass the correct parameters to our code. Eventually, someone will forget that, and then we will have one more bug ;)
class X(): ... def x(self): ... pass ... a = X() print str(a) <__main__.X instance at 0x7f8fe13d4b00> print "this is a string: %s." % a
your example make me do not need to worry about this. this is a string: <__main__.X instance at 0x7f8fe13d4b00>. In [20]: print a <__main__.X instance at 0x206c440> In [21]: print u"%s" % a <__main__.X instance at 0x206c440> -- Thanks and best regards! Sheldon Feng(???)<shaohef@linux.vnet.ibm.com> IBM Linux Technology Center
On 03/06/2014 11:46 PM, Crístian Viana wrote:
Am 06-03-2014 12:12, schrieb Sheldon:
AFAK, seems two types has no string representation, one is unicode and another is object derived from nothing. The statement if not isinstance(value, unicode), which is in the sample code above, makes sure that we will not try to convert a unicode object to string (only those who have a different type). I have forgot to tell one result I have tried: In [24]: u"fo(o( = %s" % "fo(o("
UnicodeDecodeError Traceback (most recent call last) only this case need the statement if not isinstance(value, unicode), In [23]: u"fo(o( = %s" % u"fo(o(" Out[23]: u'f\u01d2\u01d2 = f\u01d2\u01d2' not find other case. if you find, you can tell us. Thanks.
And an object derived from nothing also has a string representation. Take a look at this example:
class X(): ... def x(self): ... pass ... a = X() print str(a) <__main__.X instance at 0x7f8fe13d4b00> print "this is a string: %s." % a this is a string: <__main__.X instance at 0x7f8fe13d4b00>.
But I'm not worry about it. IMO, no one will not pass this instance of object to KimchiException as args. IMO, we should never trust that the users/developers will pass the correct parameters to our code. Eventually, someone will forget that, and then we will have one more bug ;)
-- Thanks and best regards! Sheldon Feng(???)<shaohef@linux.vnet.ibm.com> IBM Linux Technology Center
? 2014/3/6 22:53, Aline Manera ??:
On 03/06/2014 06:41 AM, Sheldon wrote:
On 03/05/2014 03:35 AM, Aline Manera wrote:
From: Aline Manera <alinefm@br.ibm.com>
The error message and all its parameters must be string. So convert error code to string in order to accomplish it.
Signed-off-by: Aline Manera <alinefm@br.ibm.com> --- src/kimchi/auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/kimchi/auth.py b/src/kimchi/auth.py index af3b610..b16f2db 100644 --- a/src/kimchi/auth.py +++ b/src/kimchi/auth.py @@ -107,7 +107,7 @@ def authenticate(username, password, service="passwd"): try: auth.authenticate() except PAM.error, (resp, code): - msg_args = {'userid': username, 'code': code} + msg_args = {'userid': username, 'code': str(code)} raise OperationFailed("KCHAUTH0001E", msg_args)
return True How about the follow change? It can avoid call str() before we raise KimchiException Everywhere. Also I think it is not good to other developers to call str explicitly Everywhere.
diff --git a/src/kimchi/exception.py b/src/kimchi/exception.py index 71a4d11..73e88ce 100644 --- a/src/kimchi/exception.py +++ b/src/kimchi/exception.py @@ -50,7 +50,7 @@ class KimchiException(Exception): translation = gettext
for key, value in args.iteritems(): - if not isinstance(value, unicode): + if isinstance(value, str): args[key] = unicode(value, 'utf-8')
return unicode(translation.gettext(text), 'utf-8') % args
What about always convert to string before to unicode?
if not isinstance(value, unicode): args[key] = unicode(*str*(value), 'utf-8')
I think it is more pythonic that: try: args[key] = unicode(value, 'utf-8') except TypeError: args[key] = unicode(*str*(value), 'utf-8')
That way we guarantee everything will be unicode in the end.
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
participants (5)
-
Aline Manera -
Crístian Viana -
Daniel H Barboza -
Sheldon -
Shu Ming