[ADD] partner_firstname for v8 #14

eLBati · 2014-07-23T17:55:49Z

because it's working as is

when creating or copying users

coveralls · 2014-07-23T22:15:07Z

Coverage increased (+8.3%) when pulling 94979df on eLBati:8.0 into bf69ae2 on OCA:8.0.

eLBati · 2014-07-23T22:35:49Z

Fixed the error occurring when creating or copying users

✅

pedrobaeza · 2014-07-23T23:19:34Z

You have a false positive on Travis. This is the error:

2014-07-23 22:14:24,970 19346 ERROR openerp_test openerp.sql_db: bad query: ALTER TABLE "res_partner" ALTER COLUMN "lastname" SET NOT NULL
Traceback (most recent call last):
File "/home/travis/build/OCA/odoo-8.0/openerp/sql_db.py", line 230, in execute
res = self._obj.execute(query, params)
IntegrityError: column "lastname" contains null values
2014-07-23 22:14:24,970 19346 WARNING openerp_test openerp.models: WARNING: unable to set column lastname of table res_partner not null !
Try to re-run: openerp-server --update=module
If it doesn't work, update records and execute manually:
ALTER TABLE res_partner ALTER COLUMN lastname SET NOT NULL
Traceback (most recent call last):
File "/home/travis/build/OCA/odoo-8.0/openerp/models.py", line 2664, in _auto_init
cr.execute('ALTER TABLE "%s" ALTER COLUMN "%s" SET NOT NULL' % (self._table, k))
File "/home/travis/build/OCA/odoo-8.0/openerp/sql_db.py", line 155, in wrapper
return f(self, *args, **kwargs)
File "/home/travis/build/OCA/odoo-8.0/openerp/sql_db.py", line 230, in execute
res = self._obj.execute(query, params)
IntegrityError: column "lastname" contains null values

To fix it, I think there must be a routine on init that fills lastname with name value.

Regards.

bwrsandman · 2014-07-23T23:38:31Z

@pedrobaeza: this is exactly what this function does. Maybe the API has changed the init functions.

Either way, it would be nice to have a logger tell us that the lastname field has been filled.

pedrobaeza · 2014-07-24T00:12:53Z

We have to find then the problem. I wouldn't merge the module until we have this fixed, because it can cause a broken DB, don't you think?

Regards.

coveralls · 2014-07-26T00:11:29Z

Coverage increased (+8.44%) when pulling 74426b4 on eLBati:8.0 into bf69ae2 on OCA:8.0.

eLBati · 2014-07-26T00:22:11Z

Traceback (most recent call last):
File "/home/travis/build/OCA/odoo-8.0/openerp/models.py", line 2649, in _auto_init
cr.execute('ALTER TABLE "%s" ALTER COLUMN "%s" SET NOT NULL' % (self._table, k))
File "/home/travis/build/OCA/odoo-8.0/openerp/sql_db.py", line 155, in wrapper
return f(self, *args, **kwargs)
File "/home/travis/build/OCA/odoo-8.0/openerp/sql_db.py", line 230, in execute
res = self._obj.execute(query, params)
IntegrityError: column "lastname" contains null values

This happens because the _auto_init runs before the init and tries to set required=True for lastname field.
Just after that, the init correctly sets the NOT NULL constraint:

2014-07-26 00:10:48,523 19332 INFO openerp_test openerp.addons.partner_firstname.partner: NOT NULL constraint for res_partner.lastname correctly set

bwrsandman · 2014-07-26T00:23:21Z

can _auto_init be overriden?
Is this an odoo 8.0 bug?

bwrsandman · 2014-07-26T00:23:55Z

Coverage increased (+8.44%)

I love seeing that! 😄

eLBati · 2014-07-26T09:59:31Z

can _auto_init be overriden?

It should be overridable, but it does a lot of things and does not have hooks. It creates tables and adds constraints, so overriding it would not solve the problem.

Anyway I don't see a big problem with this error.
The purpose of init is to make sure the NOT NULL constraint on lastname to be set.
It tries with 'lastname': fields.char("Lastname", required=True) (_auto_init) and if it fails, it adds the constraint with init. As the added log reports.
After having added the constraint, the error won't appear anymore.

bwrsandman · 2014-07-26T14:30:15Z

I am sorry but, I cannot approve this while there are errors in the logs.

rvalyi · 2014-07-26T14:47:41Z

"It should be overridable, but it does a lot of things and does not have hooks"
-> then may be it's time to propose a small refactor in official Odoo and lobby to get that in v8 while we may still have a few days to do so...

nhomar · 2014-07-26T21:54:12Z

2014-07-26 10:17 GMT-04:30 Raphaël Valyi notifications@github.com:

-> then may be it's time to propose a small refactor in official Odoo and
lobby to get that in v8 while we may still have a few days to do so...

Absolutly agreed with this point.

Saludos Cordiales

Nhomar G. Hernandez M.
+58-414-4110269
Skype: nhomar00
Web-Blog: http://geronimo.com.ve
Servicios IT: http://vauxoo.com
Linux-Counter: 467724
Correos:
nhomar@openerp.com.ve
nhomar@vauxoo.com
twitter @nhomar

hbrunn · 2014-07-28T07:59:32Z

how about 'lastname': fields.char(...., oldname='name')?

eLBati · 2014-07-29T10:43:10Z

how about 'lastname': fields.char(...., oldname='name')?

I get

2014-07-29 10:40:50,067 25164 INFO demo_8 openerp.modules.module: module partner_firstname: creating or updating database tables
2014-07-29 10:40:50,160 25164 ERROR demo_8 openerp.sql_db: Programming error: column "name" of relation "res_partner" does not exist
, in query ALTER TABLE "res_partner" ALTER COLUMN "name" DROP NOT NULL
2014-07-29 10:40:50,160 25164 CRITICAL demo_8 openerp.service.server: Failed to initialize database `demo_8`.
Traceback (most recent call last):
  File "/home/elbati/workspace/odoo/buildout_cfg-odoo-8/parts/odoo/openerp/service/server.py", line 901, in preload_registries
    registry = RegistryManager.new(dbname, update_module=update_module)
  File "/home/elbati/workspace/odoo/buildout_cfg-odoo-8/parts/odoo/openerp/modules/registry.py", line 322, in new
    openerp.modules.load_modules(registry._db, force_demo, status, update_module)
  File "/home/elbati/workspace/odoo/buildout_cfg-odoo-8/parts/odoo/openerp/modules/loading.py", line 357, in load_modules
    loaded_modules, update_module)
  File "/home/elbati/workspace/odoo/buildout_cfg-odoo-8/parts/odoo/openerp/modules/loading.py", line 263, in load_marked_modules
    loaded, processed = load_module_graph(cr, graph, progressdict, report=report, skip_modules=loaded_modules, perform_checks=perform_checks)
  File "/home/elbati/workspace/odoo/buildout_cfg-odoo-8/parts/odoo/openerp/modules/loading.py", line 163, in load_module_graph
    init_module_models(cr, package.name, models)
  File "/home/elbati/workspace/odoo/buildout_cfg-odoo-8/parts/odoo/openerp/modules/module.py", line 276, in init_module_models
    result = obj._auto_init(cr, {'module': module_name})
  File "/home/elbati/workspace/odoo/buildout_cfg-odoo-8/parts/odoo/openerp/api.py", line 204, in wrapper
    return old_api(self, *args, **kwargs)
  File "/home/elbati/workspace/odoo/buildout_cfg-odoo-8/parts/odoo/openerp/models.py", line 2582, in _auto_init
    cr.execute('ALTER TABLE "%s" ALTER COLUMN "%s" DROP NOT NULL' % (self._table, k))
  File "/home/elbati/workspace/odoo/buildout_cfg-odoo-8/parts/odoo/openerp/sql_db.py", line 155, in wrapper
    return f(self, *args, **kwargs)
  File "/home/elbati/workspace/odoo/buildout_cfg-odoo-8/parts/odoo/openerp/sql_db.py", line 230, in execute
    res = self._obj.execute(query, params)
ProgrammingError: column "name" of relation "res_partner" does not exist

eLBati · 2014-07-29T14:00:33Z

I fixed the error overriding _set_default_value_on_column instead of init: eLBati@ae4c47d

I also fixed PEP8 for the whole repo, but travis doesn't collaborate and doesn't rebuild

coveralls · 2014-07-29T14:22:03Z

Coverage increased (+8.73%) when pulling ecd45fa on eLBati:8.0 into bf69ae2 on OCA:8.0.

pedrobaeza · 2014-07-29T14:40:20Z

partner_firstname/partner.py

+            cr.execute('SELECT id FROM res_partner WHERE lastname IS NOT NULL '
+                       'Limit 1')
+            if not cr.fetchone():
+                cr.execute('UPDATE res_partner set lastname = name WHERE name '


Why don't you do it always? It will affect 0 or n rows, but it's as fast as making a select.

pedrobaeza · 2014-07-29T19:33:39Z

Please force a Travis rebuild making this in local repository in the updated PR branch:

git commit --ammend
git push <name_of_the_remote> <name_of_the_PR_branch> -f

Regards.

bwrsandman · 2014-07-29T20:07:42Z

@pedrobaeza I can restart the build without having to do any amending

Done

pedrobaeza · 2014-07-29T20:22:21Z

OK, as you want. It was only a trick I have discovered, because I don't have on some branches the possibility to make a rebuild on Travis.

eLBati · 2014-07-30T07:27:16Z

Travis connection error is still there

pedrobaeza · 2014-07-30T07:31:27Z

I don't know why is this happening. I haven't seen this error before in any repository.

eLBati · 2014-07-30T07:40:01Z

On 07/30/2014 09:31 AM, Pedro M. Baeza wrote:

I don't know why is this happening. I haven't seen this error before
in any repository.

It should be a temporary connection problem while downloading a PIP package

bwrsandman · 2014-07-30T14:20:51Z

Restarted build

coveralls · 2014-07-30T14:28:11Z

Coverage increased (+9.15%) when pulling c792e1b on eLBati:8.0 into bf69ae2 on OCA:8.0.

bwrsandman · 2014-07-30T14:42:39Z

Still errored from pip install -r /home/travis/maintainer-quality-tools/travis/requirements.txt

eLBati · 2014-07-30T14:44:52Z

I see green at https://travis-ci.org/OCA/partner-contact/builds/31171276

bwrsandman · 2014-07-30T14:46:57Z

At
~~https://travis-ci.org/OCA/partner-contact/builds/31171276#L327~~
~~I see~~

error: [Errno 104] Connection reset by peer

Storing debug log for failure in /home/travis/.pip/pip.log

The command "travis_install_nightly ${VERSION}" failed and exited with 2 during .

Browse cache error, sorry.

bwrsandman · 2014-07-30T14:49:38Z

All is well.
Thank you for your patience.
👍

bwrsandman · 2014-07-30T14:50:58Z

You sure you don't want to convert it to the new api?

https://github.com/nbessi/odoo_new_api_guideline

pedrobaeza · 2014-07-30T15:09:18Z

👍

Thanks.

nbessi · 2014-08-04T13:10:24Z

@bwrsandman I would prefer it too as it is quite a small addons, But nothing was formely decide by OCA. I have allready made this work during partner day work shop.

https://code.launchpad.net/~camptocamp/partner-contact-management/partner-first-name-new-api-v8

I should port this MP to github now that view and model are mostly frozen.

Regards

Nicolas

bwrsandman · 2014-08-04T13:59:23Z

I have a feeling that if we don't port while converting to v8 (which we often do minimalistically), we will only have modules in the new API from people who feel like doing it.

@nbessi Can your openerp-conventions be made to test for the new API this on v8s?

lmignon · 2014-08-04T15:24:56Z

partner_firstname/partner.py

+                       'SET NOT NULL')
+            _logger.info("NOT NULL constraint for "
+                         "res_partner.lastname correctly set")
+        return res



Why to not use install hooks?
odoo/odoo@4105b5f

Why to not use install hooks?
odoo/odoo@4105b5f
odoo/odoo@4105b5f

How?

Anyway, I think _set_default_value_on_column is the right place to
do it, as we are doing just that: setting a default value for the new
column lastname and adding the NOT NULL constraint on it

After reading the code in openerp.models, you're right, the set_default_value_on_column is the right way to do it since it's only called on the first installation or when upgrading the module 'ONLY' if the 'Not null' constrains is not yet applied. In proposing the 'install hooks', I wanted to avoid that this operation (_set_default_value_on_calumn) is called each time the module is upgraded.

eLBati · 2014-08-04T17:32:06Z

On 07/30/2014 04:50 PM, Sandy wrote:

You sure you don't want to convert it to the new api?

https://github.com/nbessi/odoo_new_api_guideline

I'm sure it would be good 😏

I just have no time in these days.

bwrsandman · 2014-08-04T19:26:23Z

I tired it. It is not an easy port...

Coverting create to @api.one means the api will drop the 'name' value for res users when creating related partner.

However, using the new fields, especially the compute fields are really good.

nbessi · 2014-08-05T09:10:58Z

@bwrsandman I'm working on convention on my free time.
I have made major refactoring to support both API version and have all the correct bases to add required checks. I'm working on redefining name space correctly and intergrate stuff with travis. But I do not expect a release before the end of august.

bwrsandman · 2014-08-07T14:01:07Z

We have two approves so far. Are we ready to merge or are there still objections?

JonathanNEMRY · 2014-08-07T14:02:50Z

👍

hbrunn · 2014-08-11T09:11:39Z

👍 - using the new api shouldn't be a prerequisite for porting currently, maybe at a later point.

[ADD] partner_firstname for v8

Code Cleanup and Rename rectification

[ADD] partner_firstname for v8

709daa4

because it's working as is

eLBati mentioned this pull request Jul 23, 2014

[8.0] Fix install of partner_firstname #11

Closed

[FIX] IntegrityError: column "lastname" contains null values

94979df

when creating or copying users

eLBati added 2 commits July 26, 2014 01:46

[IMP] partner_firstname - logging

032c07c

[FIX] PEP8

74426b4

eLBati added 6 commits July 29, 2014 15:21

[FIX] IntegrityError: column "lastname" contains null values

ae4c47d

[FIX] partner_firstname - some PEP8

8dc056b

[FIX] partner_firstname - PEP8

83539df

[FIX] base_location PEP8

65603cc

[FIX] passport PEP8

c600209

[FIX] base_partner_merge PEP8

ecd45fa

pedrobaeza reviewed Jul 29, 2014
View reviewed changes

[IMP] _set_default_value_on_column avoiding useless queries

c792e1b

lmignon reviewed Aug 4, 2014
View reviewed changes

hbrunn added a commit that referenced this pull request Aug 11, 2014

Merge pull request #14 from eLBati/8.0

332ca70

[ADD] partner_firstname for v8

hbrunn merged commit 332ca70 into OCA:8.0 Aug 11, 2014

JonathanNEMRY pushed a commit to acsone/partner-contact that referenced this pull request Sep 24, 2015

Merge pull request OCA#14 from parthivgls/8.0

4008a74

Code Cleanup and Rename rectification

[ADD] partner_firstname for v8 #14

[ADD] partner_firstname for v8 #14

Conversation

eLBati commented Jul 23, 2014

coveralls commented Jul 23, 2014

eLBati commented Jul 23, 2014

pedrobaeza commented Jul 23, 2014

bwrsandman commented Jul 23, 2014

pedrobaeza commented Jul 24, 2014

coveralls commented Jul 26, 2014

eLBati commented Jul 26, 2014

bwrsandman commented Jul 26, 2014

bwrsandman commented Jul 26, 2014

eLBati commented Jul 26, 2014

bwrsandman commented Jul 26, 2014

rvalyi commented Jul 26, 2014

nhomar commented Jul 26, 2014

hbrunn commented Jul 28, 2014

eLBati commented Jul 29, 2014

eLBati commented Jul 29, 2014

coveralls commented Jul 29, 2014

pedrobaeza Jul 29, 2014

Choose a reason for hiding this comment

pedrobaeza commented Jul 29, 2014

bwrsandman commented Jul 29, 2014

pedrobaeza commented Jul 29, 2014

eLBati commented Jul 30, 2014

pedrobaeza commented Jul 30, 2014

eLBati commented Jul 30, 2014

bwrsandman commented Jul 30, 2014

coveralls commented Jul 30, 2014

bwrsandman commented Jul 30, 2014

eLBati commented Jul 30, 2014

bwrsandman commented Jul 30, 2014

bwrsandman commented Jul 30, 2014

bwrsandman commented Jul 30, 2014

pedrobaeza commented Jul 30, 2014

nbessi commented Aug 4, 2014

bwrsandman commented Aug 4, 2014

lmignon Aug 4, 2014

Choose a reason for hiding this comment

eLBati Aug 4, 2014

Choose a reason for hiding this comment

lmignon Aug 4, 2014

Choose a reason for hiding this comment

eLBati commented Aug 4, 2014

bwrsandman commented Aug 4, 2014

nbessi commented Aug 5, 2014

bwrsandman commented Aug 7, 2014

JonathanNEMRY commented Aug 7, 2014

hbrunn commented Aug 11, 2014