[Kimchi-devel] [PATCH] issue #330: Properly log the error message when login fails

[Sheldon]

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 at 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 at 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 at linux.vnet.ibm.com>
IBM Linux Technology Center

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ovirt.org/pipermail/kimchi-devel/attachments/20140306/7237ba6e/attachment.html>