Merge lp:~jamesmikedupont/openobject-server/openobject-server into lp:openobject-server

[James Michael DuPont]

Running of pylint
https://bugs.launchpad.net/openobject-server/+bug/1068124

[lp:~jamesmikedupont/openobject-server/openobject-server]

4496. By James Michael DuPont on 2012-11-08

merge

4497. By James Michael DuPont on 2012-11-08

pylint

[Xavier (Open ERP) (xmo-deactivatedaccount)]

I can't say I'm fond of the way many lines are broken to fit in 80chars. Not the fitting in 80chars thing (that's great), but the way it's done e.g.

    sys.stderr.write("Running as user 'root' is a security risk, aborting.\n")

to

    sys.stderr.write("Running as user 'root'"
                     " is a security risk, aborting.\n")

I think would make more sense (and look better) as

    sys.stderr.write(
        "Running as user 'root' is a security risk, aborting.\n")

and

       db, registry = openerp.pooler.get_db_and_pool(dbname, update_module=openerp.tools.config['init'] or openerp.tools.config['update'], pooljobs=False)

to

       _db, registry = \
           openerp.pooler.get_db_and_pool(dbname, \
                                              update_module=openerp.tools.config['init'] \
                                              or openerp.tools.config['update'], \
                                              pooljobs=False)

is downright weird, why not e.g.

        to_update = openerp.tools.config['init'] \
                 or openerp.tools.config['update']
        _db, registry = openerp.pooler.get_db_and_pool(
            dbname, update_module=to_update, pooljobs=False)

this kind of wonky line-breaking seems fairly common and hampers readablility more than it helps.

review: Needs Fixing

[James Michael DuPont (jamesmikedupont)]

ok fair enough, I will rework it when I have time.

On Tue, Nov 13, 2012 at 3:39 AM, Xavier (Open ERP) <email address hidden> wrote:
> Review: Needs Fixing
>
> I can't say I'm fond of the way many lines are broken to fit in 80chars. Not the fitting in 80chars thing (that's great), but the way it's done e.g.
>
> sys.stderr.write("Running as user 'root' is a security risk, aborting.\n")
>
> to
>
> sys.stderr.write("Running as user 'root'"
> " is a security risk, aborting.\n")
>
> I think would make more sense (and look better) as
>
> sys.stderr.write(
> "Running as user 'root' is a security risk, aborting.\n")
>
> and
>
> db, registry = openerp.pooler.get_db_and_pool(dbname, update_module=openerp.tools.config['init'] or openerp.tools.config['update'], pooljobs=False)
>
> to
>
> _db, registry = \
> openerp.pooler.get_db_and_pool(dbname, \
> update_module=openerp.tools.config['init'] \
> or openerp.tools.config['update'], \
> pooljobs=False)
>
> is downright weird, why not e.g.
>
> to_update = openerp.tools.config['init'] \
> or openerp.tools.config['update']
> _db, registry = openerp.pooler.get_db_and_pool(
> dbname, update_module=to_update, pooljobs=False)
>
> this kind of wonky line-breaking seems fairly common and hampers readablility more than it helps.
> --
> https://code.launchpad.net/~jamesmikedupont/openobject-server/openobject-server/+merge/133440
> You are the owner of lp:~jamesmikedupont/openobject-server/openobject-server.

--
James Michael DuPont
Member of Free Libre Open Source Software Kosova http://flossk.org
Saving wikipedia(tm) articles from deletion http://SpeedyDeletion.wikia.com
Contributor FOSM, the CC-BY-SA map of the world http://fosm.org
Mozilla Rep https://reps.mozilla.org/u/h4ck3rm1k3
Free Software Foundation Europe Fellow http://fsfe.org/support/?h4ck3rm1k3

[Antony Lesuisse (OpenERP) (al-openerp)]

please rebase on trunk

[James Michael DuPont (jamesmikedupont)]

I have not had time to do this,
thanks
mike

On Tue, Dec 18, 2012 at 10:02 AM, Antony Lesuisse (OpenERP)
<email address hidden>wrote:

> please rebase on trunk
> --
>
> https://code.launchpad.net/~jamesmikedupont/openobject-server/openobject-server/+merge/133440
> You are the owner of
> lp:~jamesmikedupont/openobject-server/openobject-server.
>

--
James Michael DuPont
Member of Free Libre Open Source Software Kosova http://flossk.org
Saving wikipedia(tm) articles from deletion http://SpeedyDeletion.wikia.com
Contributor FOSM, the CC-BY-SA map of the world http://fosm.org
Mozilla Rep https://reps.mozilla.org/u/h4ck3rm1k3
Free Software Foundation Europe Fellow http://fsfe.org/support/?h4ck3rm1k3

Unmerged revisions

4497. By James Michael DuPont on 2012-11-08

pylint

4496. By James Michael DuPont on 2012-11-08

merge

4495. By James Michael DuPont on 2012-10-18

clean

=== modified file 'openerp-server'
--- openerp-server	2012-10-29 17:33:21 +0000
+++ openerp-server	2012-11-08 11:45:27 +0000
@@ -19,7 +19,7 @@
 #    along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
 ##############################################################################
-
+# pylint: disable=C0103
 """
 OpenERP - Server
 OpenERP is an ERP+CRM program for small and medium businesses.
@@ -50,7 +50,8 @@
     if os.name == 'posix':
         import pwd
         if pwd.getpwuid(os.getuid())[0] == 'root' :
-            sys.stderr.write("Running as user 'root' is a security risk, aborting.\n")
+            sys.stderr.write("Running as user 'root'"
+                             "is a security risk, aborting.\n")
             sys.exit(1)
 
 def check_postgres_user():
@@ -60,7 +61,8 @@
     """
     config = openerp.tools.config
     if config['db_user'] == 'postgres':
-        sys.stderr.write("Using the database user 'postgres' is a security risk, aborting.")
+        sys.stderr.write("Using the database user 'postgres'"
+                         "is a security risk, aborting.")
         sys.exit(1)
 
 def report_configuration():
@@ -91,7 +93,11 @@
 def preload_registry(dbname):
     """ Preload a registry, and start the cron."""
     try:
-        db, registry = openerp.pooler.get_db_and_pool(dbname, update_module=openerp.tools.config['init'] or openerp.tools.config['update'], pooljobs=False)
+        _db, registry = \
+            openerp.pooler.get_db_and_pool(dbname, \
+                                               update_module=openerp.tools.config['init'] \
+                                               or openerp.tools.config['update'], \
+                                               pooljobs=False)
 
         # jobs will start to be processed later, when openerp.cron.start_master_thread() is called by openerp.service.start_services()
         registry.schedule_cron_jobs()
@@ -102,7 +108,7 @@
     """ Preload a registry, possibly run a test file, and start the cron."""
     try:
         config = openerp.tools.config
-        db, registry = openerp.pooler.get_db_and_pool(dbname, update_module=config['init'] or config['update'], pooljobs=False)
+        db, _registry = openerp.pooler.get_db_and_pool(dbname, update_module=config['init'] or config['update'], pooljobs=False)
         cr = db.cursor()
         _logger.info('loading test file %s', test_file)
         openerp.tools.convert_yaml_import(cr, 'base', file(test_file), 'test', {}, 'test', True)
@@ -112,6 +118,7 @@
         _logger.exception('Failed to initialize database `%s` and run test file `%s`.', dbname, test_file)
 
 def export_translation():
+    """ export the translation file to the file specified by translate_out"""
     config = openerp.tools.config
     dbname = config['db_name']
 
@@ -133,6 +140,7 @@
     _logger.info('translation file written successfully')
 
 def import_translation():
+    """ Import the translations back in """
     config = openerp.tools.config
     context = {'overwrite': config["overwrite_existing_translations"]}
     dbname = config['db_name']
@@ -147,7 +155,7 @@
 # below. This variable is monitored by ``quit_on_signals()``.
 quit_signals_received = 0
 
-def signal_handler(sig, frame):
+def signal_handler(_sig, _frame):
     """ Signal handler: exit ungracefully on the second handled signal.
 
     :param sig: the signal number
@@ -160,7 +168,7 @@
         sys.stderr.write("Forced shutdown.\n")
         os._exit(0)
 
-def dumpstacks(sig, frame):
+def dumpstacks(_sig, _frame):
     """ Signal handler: dump a stack trace for each existing thread."""
     # code from http://stackoverflow.com/questions/132058/getting-stack-trace-from-a-running-python-application#answer-2569696
     # modified for python 2.5 compatibility
@@ -187,7 +195,9 @@
         map(lambda sig: signal.signal(sig, signal_handler), SIGNALS)
         signal.signal(signal.SIGQUIT, dumpstacks)
     elif os.name == 'nt':
+# pylint: disable=F0401
         import win32api
+# pylint: enable=F0401
         win32api.SetConsoleCtrlHandler(lambda sig: signal_handler(sig, None), 1)
 
 def quit_on_signals():
@@ -213,12 +223,14 @@
     sys.exit(0)
 
 def configure_babel_localedata_path():
-    # Workaround: py2exe and babel.
+    """ Workaround: py2exe and babel. """
     if hasattr(sys, 'frozen'):
         import babel
         babel.localedata._dirname = os.path.join(os.path.dirname(sys.executable), 'localedata')
 
 def main():
+    """ the main routine for OPENERP """
+
     os.environ["TZ"] = "UTC"
 
     check_root_user()

=== modified file 'openerp/service/http_server.py'
--- openerp/service/http_server.py	2012-09-11 15:07:24 +0000
+++ openerp/service/http_server.py	2012-11-08 11:45:27 +0000
@@ -34,7 +34,8 @@
 
     The OpenERP server defines a single instance of a HTTP server, listening at
     the standard 8069, 8071 ports (well, it is 2 servers, and ports are 
-    configurable, of course). This "single" server then uses a `MultiHTTPHandler`
+    configurable, of course). 
+    This "single" server then uses a `MultiHTTPHandler`
     to dispatch requests to the appropriate channel protocol, like the XML-RPC,
     static HTTP, DAV or other.
 """
@@ -44,51 +45,28 @@
 import urllib
 import os
 import logging
+from  openerp.service.security import security 
+from openerp.service.websrv_lib import FixSendError, \
+    HttpOptions, HTTPHandler, reg_http_service, AuthProvider,\
+    AuthRejectedExc, AuthRequiredExc
 
-from websrv_lib import *
 import openerp.tools as tools
-
-try:
-    import fcntl
-except ImportError:
-    fcntl = None
-
-try:
-    from ssl import SSLError
-except ImportError:
-    class SSLError(Exception): pass
-
+    
 _logger = logging.getLogger(__name__)
 
-# TODO delete this for 6.2, it is still needed for 6.1.
-class HttpLogHandler:
-    """ helper class for uniform log handling
-    Please define self._logger at each class that is derived from this
-    """
-    _logger = None
-    
-    def log_message(self, format, *args):
-        self._logger.debug(format % args) # todo: perhaps other level
-
-    def log_error(self, format, *args):
-        self._logger.error(format % args)
-        
-    def log_exception(self, format, *args):
-        self._logger.exception(format, *args)
-
-    def log_request(self, code='-', size='-'):
-        self._logger.debug('"%s" %s %s',
-            self.requestline, str(code), str(size))
-
-class StaticHTTPHandler(HttpLogHandler, FixSendError, HttpOptions, HTTPHandler):
+
+class StaticHTTPHandler( FixSendError, HttpOptions, HTTPHandler):
+    """ The static http handler"""
     _logger = logging.getLogger(__name__)
 
     _HTTP_OPTIONS = { 'Allow': ['OPTIONS', 'GET', 'HEAD'] }
 
-    def __init__(self,request, client_address, server):
-        HTTPHandler.__init__(self,request,client_address,server)
+    def __init__(self, request, client_address, server):
+        HTTPHandler.__init__(self, request, client_address, server)
         document_root = tools.config.get('static_http_document_root', False)
-        assert document_root, "Please specify static_http_document_root in configuration, or disable static-httpd!"
+        assert document_root, \
+        "Please specify static_http_document_root in configuration, "\
+        "or disable static-httpd!"
         self.__basepath = document_root
 
     def translate_path(self, path):
@@ -100,23 +78,27 @@
 
         """
         # abandon query parameters
-        path = path.split('?',1)[0]
-        path = path.split('#',1)[0]
+        path = path.split('?', 1)[0]
+        path = path.split('#', 1)[0]
         path = posixpath.normpath(urllib.unquote(path))
         words = path.split('/')
         words = filter(None, words)
         path = self.__basepath
         for word in words:
-            if word in (os.curdir, os.pardir): continue
+            if word in (os.curdir, os.pardir): 
+                continue
             path = os.path.join(path, word)
         return path
 
 def init_static_http():
+    """setup the static file http handler."""
     if not tools.config.get('static_http_enable', False):
         return
     
     document_root = tools.config.get('static_http_document_root', False)
-    assert document_root, "Document root must be specified explicitly to enable static HTTP service (option --static-http-document-root)"
+    assert document_root, \
+    "Document root must be specified explicitly to enable static HTTP service" \
+    " (option --static-http-document-root)"
     
     base_path = tools.config.get('static_http_url_prefix', '/')
     
@@ -124,35 +106,39 @@
     
     _logger.info("Registered HTTP dir %s for %s", document_root, base_path)
 
-import security
+
 
 class OpenERPAuthProvider(AuthProvider):
     """ Require basic authentication."""
-    def __init__(self,realm='OpenERP User'):
-        self.realm = realm
+    def __init__(self, realm='OpenERP User'):
+        AuthProvider.__init__(self, realm)
         self.auth_creds = {}
         self.auth_tries = 0
         self.last_auth = None
 
     def authenticate(self, db, user, passwd, client_address):
         try:
-            uid = security.login(db,user,passwd)
+            uid = security.login(db, user, passwd)
             if uid is False:
                 return False
             return (user, passwd, db, uid)
-        except Exception,e:
-            _logger.debug("Fail auth: %s" % e )
-            return False
-
-    def checkRequest(self,handler,path, db=False):        
-        auth_str = handler.headers.get('Authorization',False)
+        except AuthRejectedExc, e:
+            _logger.debug("Fail auth: %s" % e )
+            return False
+        except AuthRequiredExc, e:
+            _logger.debug("Fail auth: %s" % e )
+            return False
+
+
+    def checkRequest(self, handler, path, db=False):        
+        auth_str = handler.headers.get('Authorization', False)
         try:
             if not db:
                 db = handler.get_db_from_path(path)
-        except Exception:
+        except Exception: #FIXME! what exception is really thrown here?
             if path.startswith('/'):
                 path = path[1:]
-            psp= path.split('/')
+            psp = path.split('/')
             if len(psp)>1:
                 db = psp[0]
             else:
@@ -162,10 +148,12 @@
         if self.auth_creds.get(db):
             return True 
         if auth_str and auth_str.startswith('Basic '):
-            auth_str=auth_str[len('Basic '):]
-            (user,passwd) = base64.decodestring(auth_str).split(':')
-            _logger.info("Found user=\"%s\", passwd=\"***\" for db=\"%s\"", user, db)
-            acd = self.authenticate(db,user,passwd,handler.client_address)
+            auth_str = auth_str[len('Basic '):]
+            (user, passwd) = base64.decodestring(auth_str).split(':')
+            _logger.info(\
+             "Found user=\"%s\", passwd=\"***\" for db=\"%s\"", \
+             user, db)
+            acd = self.authenticate(db, user, passwd, handler.client_address)
             if acd != False:
                 self.auth_creds[db] = acd
                 self.last_auth = db

=== modified file 'openerp/service/websrv_lib.py'
--- openerp/service/websrv_lib.py	2012-02-12 11:45:09 +0000
+++ openerp/service/websrv_lib.py	2012-11-08 11:45:27 +0000
@@ -52,13 +52,13 @@
     def __init__(self,realm):
         self.realm = realm
 
-    def authenticate(self, user, passwd, client_address):
+    def authenticate(self, db, user, passwd, client_address):
         return False
 
     def log(self, msg):
         print msg
 
-    def checkRequest(self,handler,path = '/'):
+    def checkRequest(self, handler, path = '/', db=False):
         """ Check if we are allowed to process that request
         """
         pass