[REF] Use the contextlib.closing function instead of a couple of try/catch for

the release of the database cursor.

bzr revid: stw@openerp.com-20140109093258-vmr1a3gaxruo0psp

openerp/cli/server.py

@@ -85,10 +85,9 @@ def setup_pid_file():
     """
     config = openerp.tools.config
     if config['pidfile']:
-        fd = open(config['pidfile'], 'w')
+        with open(config['pidfile'], 'w') as fd:
-        pidtext = "%d" % (os.getpid())
+            pidtext = "%d" % (os.getpid())
-        fd.write(pidtext)
+            fd.write(pidtext)
-        fd.close()
 
 def preload_registry(dbname):
     """ Preload a registry, and start the cron."""
@@ -173,7 +172,7 @@ def main(args):
     if config['workers']:
         openerp.multi_process = True
 
-    # preload registryies, needed for -u --stop_after_init
+    # preload registries, needed for -u --stop_after_init
     rc = 0
     if config['db_name']:
         for dbname in config['db_name'].split(','):

openerp/service/db.py

@@ -6,6 +6,7 @@ import logging
 import os
 import threading
 import traceback
+from contextlib import contextmanager, closing
 
 import openerp
 from openerp import SUPERUSER_ID
@@ -24,24 +25,17 @@ self_id_protect = threading.Semaphore()
 # This should be moved to openerp.modules.db, along side initialize().
 def _initialize_db(id, db_name, demo, lang, user_password):
     try:
-        cr = None
+        self_actions[id]['progress'] = 0
-        try:
+        db = openerp.sql_db.db_connect(db_name)
-            self_actions[id]['progress'] = 0
+        with closing(db.cursor()) as cr:
-            cr = openerp.sql_db.db_connect(db_name).cursor()
             openerp.modules.db.initialize(cr) # TODO this should be removed as it is done by RegistryManager.new().
             openerp.tools.config['lang'] = lang
             cr.commit()
-        finally:
-            if cr:
-                cr.close()
-                cr = None
 
         registry = openerp.modules.registry.RegistryManager.new(
             db_name, demo, self_actions[id], update_module=True)
 
-        try:
+        with closing(db.cursor()) as cr:
-            cr = openerp.sql_db.db_connect(db_name).cursor()
-
             if lang:
                 modobj = registry['ir.module.module']
                 mids = modobj.search(cr, SUPERUSER_ID, [('state', '=', 'installed')])
@@ -54,9 +48,7 @@ def _initialize_db(id, db_name, demo, lang, user_password):
             cr.execute('SELECT login, password FROM res_users ORDER BY login')
             self_actions[id].update(users=cr.dictfetchall(), clean=True)
             cr.commit()
-        finally:
+
-            if cr:
-                cr.close()
     except Exception, e:
         self_actions[id].update(clean=False, exception=e)
         _logger.exception('CREATE DATABASE failed:')
@@ -81,19 +73,15 @@ def dispatch(method, params):
 
 def _create_empty_database(name):
     db = openerp.sql_db.db_connect('postgres')
-    cr = db.cursor()
+    with closing(db.cursor()) as cr:
-    chosen_template = openerp.tools.config['db_template']
+        chosen_template = openerp.tools.config['db_template']
-    cr.execute("""SELECT datname 
+        cr.execute("SELECT datname FROM pg_database WHERE datname = %s",
-                          FROM pg_database
+                   (name,))
-                          WHERE datname = %s """,
+        if cr.fetchall():
-                       (name,))
+            raise openerp.exceptions.Warning(" %s database already exists!" % name )
-    if cr.fetchall():
+        else:
-        raise openerp.exceptions.Warning(" %s database already exists!" % name )
+            cr.autocommit(True) # avoid transaction block
-    try:
+            cr.execute("""CREATE DATABASE "%s" ENCODING 'unicode' TEMPLATE "%s" """ % (name, chosen_template))
-        cr.autocommit(True) # avoid transaction block
-        cr.execute("""CREATE DATABASE "%s" ENCODING 'unicode' TEMPLATE "%s" """ % (name, chosen_template))
-    finally:
-        cr.close()
 
 def exp_create(db_name, demo, lang, user_password='admin'):
     self_id_protect.acquire()
@@ -132,12 +120,9 @@ def exp_duplicate_database(db_original_name, db_name):
     _logger.info('Duplicate database `%s` to `%s`.', db_original_name, db_name)
     openerp.sql_db.close_db(db_original_name)
     db = openerp.sql_db.db_connect('postgres')
-    cr = db.cursor()
+    with closing(db.cursor()) as cr:
-    try:
         cr.autocommit(True) # avoid transaction block
         cr.execute("""CREATE DATABASE "%s" ENCODING 'unicode' TEMPLATE "%s" """ % (db_name, db_original_name))
-    finally:
-        cr.close()
     return True
 
 def exp_get_progress(id):
@@ -166,9 +151,8 @@ def exp_drop(db_name):
     openerp.sql_db.close_db(db_name)
 
     db = openerp.sql_db.db_connect('postgres')
-    cr = db.cursor()
+    with closing(db.cursor()) as cr:
-    cr.autocommit(True) # avoid transaction block
+        cr.autocommit(True) # avoid transaction block
-    try:
         # Try to terminate all other connections that might prevent
         # dropping the database
         try:
@@ -192,8 +176,6 @@ def exp_drop(db_name):
             raise Exception("Couldn't drop database %s: %s" % (db_name, e))
         else:
             _logger.info('DROP DB: %s', db_name)
-    finally:
-        cr.close()
     return True
 
 @contextlib.contextmanager
@@ -290,9 +272,8 @@ def exp_rename(old_name, new_name):
     openerp.sql_db.close_db(old_name)
 
     db = openerp.sql_db.db_connect('postgres')
-    cr = db.cursor()
+    with closing(db.cursor()) as cr:
-    cr.autocommit(True) # avoid transaction block
+        cr.autocommit(True) # avoid transaction block
-    try:
         try:
             cr.execute('ALTER DATABASE "%s" RENAME TO "%s"' % (old_name, new_name))
         except Exception, e:
@@ -304,8 +285,6 @@ def exp_rename(old_name, new_name):
                 os.rename(os.path.join(fs, old_name), os.path.join(fs, new_name))
 
             _logger.info('RENAME DB: %s -> %s', old_name, new_name)
-    finally:
-        cr.close()
     return True
 
 def exp_db_exist(db_name):
@@ -318,8 +297,7 @@ def exp_list(document=False):
     chosen_template = openerp.tools.config['db_template']
     templates_list = tuple(set(['template0', 'template1', 'postgres', chosen_template]))
     db = openerp.sql_db.db_connect('postgres')
-    cr = db.cursor()
+    with closing(db.cursor()) as cr:
-    try:
         try:
             db_user = openerp.tools.config["db_user"]
             if not db_user and os.name == 'posix':
@@ -336,8 +314,6 @@ def exp_list(document=False):
             res = [str(name) for (name,) in cr.fetchall()]
         except Exception:
             res = []
-    finally:
-        cr.close()
     res.sort()
     return res
 

openerp/service/model.py

@@ -10,6 +10,7 @@ import time
 import openerp
 from openerp.tools.translate import translate
 from openerp.osv.orm import except_orm
+from contextlib import contextmanager
 
 import security
 
@@ -159,41 +160,36 @@ def execute_cr(cr, uid, obj, method, *args, **kw):
 def execute_kw(db, uid, obj, method, args, kw=None):
     return execute(db, uid, obj, method, *args, **kw or {})
 
+@contextmanager
+def closing_cr_and_commit(cr):
+    try:
+        yield cr
+        cr.commit()
+    except Exception:
+        cr.rollback()
+        raise
+    finally:
+        cr.close()
+
 @check
 def execute(db, uid, obj, method, *args, **kw):
     threading.currentThread().dbname = db
-    cr = openerp.registry(db).db.cursor()
+    with closing_cr_and_commit(openerp.registry(db).db.cursor()) as cr:
-    try:
+        if method.startswith('_'):
-        try:
+            raise except_orm('Access Denied', 'Private methods (such as %s) cannot be called remotely.' % (method,))
-            if method.startswith('_'):
+        res = execute_cr(cr, uid, obj, method, *args, **kw)
-                raise except_orm('Access Denied', 'Private methods (such as %s) cannot be called remotely.' % (method,))
+        if res is None:
-            res = execute_cr(cr, uid, obj, method, *args, **kw)
+            _logger.warning('The method %s of the object %s can not return `None` !', method, obj)
-            if res is None:
+        return res
-                _logger.warning('The method %s of the object %s can not return `None` !', method, obj)
-            cr.commit()
-        except Exception:
-            cr.rollback()
-            raise
-    finally:
-        cr.close()
-    return res
 
 def exec_workflow_cr(cr, uid, obj, signal, *args):
     res_id = args[0]
     return execute_cr(cr, uid, obj, 'signal_workflow', [res_id], signal)[res_id]
 
+
 @check
 def exec_workflow(db, uid, obj, signal, *args):
-    cr = openerp.registry(db).db.cursor()
+    with closing_cr_and_commit(openerp.registry(db).db.cursor()) as cr:
-    try:
+        return exec_workflow_cr(cr, uid, obj, signal, *args)
-        try:
-            res = exec_workflow_cr(cr, uid, obj, signal, *args)
-            cr.commit()
-        except Exception:
-            cr.rollback()
-            raise
-    finally:
-        cr.close()
-    return res
 
 # vim:expandtab:smartindent:tabstop=4:softtabstop=4:shiftwidth=4: