[Kimchi-devel] [PATCH V3 1/5] Fix 'disk full' issue: Change objectstore exception handling

[Rodrigo Trujillo]

On 04/02/2014 03:33 AM, Royce Lv wrote:
> On 2014年04月01日 20:47, Rodrigo Trujillo wrote:
>> On 04/01/2014 04:04 AM, Royce Lv wrote:
>>> On 2014年03月29日 00:40, Rodrigo Trujillo wrote:
>>>> This patch changes __exit__ function of objectstore in order to log 
>>>> the
>>>> error information. It also changes the return to make python raise the
>>>> exception again, which will be caught by the functions that are using
>>>> the objectstore.
>>>>
>>>> Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo at linux.vnet.ibm.com>
>>>> ---
>>>>   src/kimchi/objectstore.py | 7 +++++++
>>>>   1 file changed, 7 insertions(+)
>>>>
>>>> diff --git a/src/kimchi/objectstore.py b/src/kimchi/objectstore.py
>>>> index d960ca9..b1c1bdd 100644
>>>> --- a/src/kimchi/objectstore.py
>>>> +++ b/src/kimchi/objectstore.py
>>>> @@ -19,6 +19,7 @@
>>>>   import json
>>>>   import sqlite3
>>>>   import threading
>>>> +import traceback
>>>>
>>>>
>>>>   try:
>>>> @@ -29,6 +30,7 @@ except ImportError:
>>>>
>>>>   from kimchi import config
>>>>   from kimchi.exception import NotFoundError
>>>> +from kimchi.utils import kimchi_log
>>>>
>>>>
>>>>   class ObjectStoreSession(object):
>>>> @@ -116,3 +118,8 @@ class ObjectStore(object):
>>>>
>>>>       def __exit__(self, type, value, tb):
>>>>           self._lock.release()
>>>> +        if type is not None and issubclass(type, 
>>>> sqlite3.DatabaseError):
>>>> +                # Logs the error and return False, which makes 
>>>> __exit__ raise
>>>> +                # exception again
>>>> +                kimchi_log.error(traceback.format_exc())
>>>> +                return False
>>> I appreciate the idea of this patch.
>>> Howerver, I wrote a demo and it seems if I don't explicitly return 
>>> True, it raises error as its default behavior
>>> And as __exit__ accepted tb as its param, instead of format a 
>>> traceback for ourselves,
>>> we need to use the passed in tb.
>>> And I'm not sure why we just raise DBerror?
>>>
>>
>> Hi Royce,
>>
>> yeap, the __exit__ is a function of "with" command that will always 
>> be called in the end of "with" command, or when an error happens.
>> If "type, value, tb" are None, means everything went ok in "with". 
>> Otherwise, __exit__ is going to re-raise the exception received in 
>> function end or re-raise when its code return a False.
>> My idea here is: the  __exit__ of objectstore should log only 
>> database errors... so, I check for this type of exception, log and 
>> make sure the exception is re-raised (the "return False" is kind of 
>> redundant, but its ok).
>> Any other problem/exception generated by "with" is going to re-raised 
>> without treatment. Python internally checks if "type, value and tb" 
>> are None, if not, it re-raises in the end... so, for instance, 
>> "NotFoundError" exception (raised when an item is not found in the 
>> database) are always returned to "with" ... which is going to deal 
>> with it.
>>
>> Let me know if its more clear now.
> So idea is to distinguish DBError and normal error, just log dberror, 
> and raise both.I think it's reasonable.
> I mean, the 'tb' argument passed to 'exit' function is traceback 
> object when exception raised,this helps us locate error point, we use 
> that instead of format an excecution stack on the exit point. Does 
> that make sense?

No, does not make sense. I am not understanding your point here.
Notice. I did not change anything in the __exit__ behaviour, I am just 
logging db error.  What is the problem on this ?

>>
>>
>>
>>> _______________________________________________
>>> Kimchi-devel mailing list
>>> Kimchi-devel at ovirt.org
>>> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>>
>
> _______________________________________________
> Kimchi-devel mailing list
> Kimchi-devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/kimchi-devel