savoirfairelinux-openerp team mailing list archive

[Nicolas Bessi - Camptocamp <nicolas.bessi@xxxxxxxxxxxxxx>]

Review: Needs Fixing code review, no tests

Hello,

Thanks for the revirew

Some comments below:

Please use explicit relative import: from . import

in __openerp__ remove the init key please it is unused

in base_external_source.py:

just a details but the order of import is not standard:
-atop should be standard lib call, 
-below external used module, 
-finally localy imported module

please use OpenERP/community convention to for the model:
from openerp.osv import orm, fields

class base_external_dbsource(orm.Model):

line 71: back slash is not needed
line 82 back slash an + is not needed

in conn_open
            if '%s' not in data.conn_string:
                connStr += ';PWD=%s'
I'm not sure this is a judicious choice.
What if password set in FREETDS config files

In function connexion_test you raise an exception to said that connection works. 
It will be better to return an ir.act_windows.

A quick pass to a lynter to fix PEP8 will be nice.

In demo data used for yaml test

        <record model="base.external.dbsource" id="demo_postgre">
            <field name="name">PostgreSQL local</field>
            <field name="conn_string">dbname='postgres' password=%s</field>
            <field name="password">postgresql</field>
            <field name="connector">postgresql</field>
        </record>

Depends on postgres configuration. I have no better idea but a comment in test should be made to validate this first.


Thanks fro the patch. I will look if I can integrate it in connector_odbc. 

Regards

Nicolas



-- 
https://code.launchpad.net/~savoirfairelinux-openerp/server-env-tools/7.0-modules-from-openobject-extension/+merge/183539
Your team Savoir-faire Linux' OpenERP is subscribed to branch lp:~savoirfairelinux-openerp/server-env-tools/7.0-modules-from-openobject-extension.