savoirfairelinux-openerp team mailing list archive
-
savoirfairelinux-openerp team
-
Mailing list archive
-
Message #00306
Re: lp:~savoirfairelinux-openerp/server-env-tools/7.0-modules-from-openobject-extension into lp:server-env-tools
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.
References