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

[Royce Lv]

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?
>
>
>
>> _______________________________________________
>> Kimchi-devel mailing list
>> Kimchi-devel at ovirt.org
>> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>