Merge lp:~sebastien.beau/openerp-connector/openerp-connector-fix-useless-fire-sparse-fields into lp:~openerp-connector-core-editors/openerp-connector/7.0

Proposed by Sébastien BEAU - http://www.akretion.com
Status: Needs review
Proposed branch: lp:~sebastien.beau/openerp-connector/openerp-connector-fix-useless-fire-sparse-fields
Merge into: lp:~openerp-connector-core-editors/openerp-connector/7.0
Diff against target: 16 lines (+4/-1)
1 file modified
connector/producer.py (+4/-1)
To merge this branch: bzr merge lp:~sebastien.beau/openerp-connector/openerp-connector-fix-useless-fire-sparse-fields
Reviewer Review Type Date Requested Status
Guewen Baconnier @ Camptocamp code review, no test Needs Fixing
Review via email: mp+175054@code.launchpad.net

Description of the change

Hi
When we write in a sparse fields (it's inherit the function fields) OpenERP will write into the serialized fields.
This mean that if I update X sparse fields on the product, the producer will fire X+1 even and so X+1 job.

The easy way to fix it is to not fire an even if the key updated is only the serialized fields.

What do you think?

Thanks

To post a comment you must log in.
Revision history for this message
Guewen Baconnier @ Camptocamp (gbaconnier-c2c) wrote :

Hi,

Thanks for your MP.

Can you add the test as well please (for a write in a serialized field too)?

Could I propose to rename 'keys' to 'fields'?

I think we can have a more clever fix for not much work. I'm maybe wrong without investigating further, but I'm sure you'll stop me in such case ;-)

If we write directly in a serialized field, we know the fields we are writing on.
I would propose to:
Fire vals.keys() + serialized.keys(), unless if '__do_not_fire_serialized' is present in the context. '__do_not_fire_serialized' has to be put in the context given to the `write_original` method.

Thus,
* if we write directly to sparse fields -> ok because the many writes on the serialized will be skipped due to the presence of the sentinel '__do_not_fire_serialized'
* If we write directly to a serialized field -> ok because the fields keys will be present in the first fire

review: Needs Fixing (code review, no test)
Revision history for this message
Guewen Baconnier @ Camptocamp (gbaconnier-c2c) wrote :

> <...>
> Fire vals.keys() + serialized.keys(), unless if '__do_not_fire_serialized' is present in the context. >'__do_not_fire_serialized' has to be put in the context given to the

Should be actually something as follows :
    fields = vals.keys()
    for each serialized field: # pseudo code
        fields = set(vals.keys()).update(serialized.keys())
        fields.remove(field_serialized)
where serialized is the the dict of serialized vals and field_serialized the name of the field

> <...>
>
> Thus,
> * if we write directly to sparse fields -> ok because the many writes on the
> serialized will be skipped due to the presence of the sentinel
> '__do_not_fire_serialized'
> * If we write directly to a serialized field -> ok because the fields keys
> will be present in the first fire

Hmm I inversed some things here, but I think you understood me.

Note that your fix is failing if we update 2 fields where 1 of them is a serialized.

When I

Revision history for this message
Guewen Baconnier @ Camptocamp (gbaconnier-c2c) wrote :

This project is now hosted on https://github.com/OCA/connector. Please move your proposal there if you still want to merge it once fixed. This guide may help you https://github.com/OCA/maintainers-tools/wiki/How-to-move-a-Merge-Proposal-to-GitHub

Unmerged revisions

579. By Sébastien BEAU - http://www.akretion.com

[FIX] fix useless fire on when writing on a sparse fields

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'connector/producer.py'
2--- connector/producer.py 2013-04-23 13:27:22 +0000
3+++ connector/producer.py 2013-07-16 15:57:28 +0000
4@@ -55,9 +55,12 @@
5 ids = [ids]
6 session = ConnectorSession(cr, uid, context=context)
7 if on_record_write.has_consumer_for(session, self._name):
8+ keys = vals.keys()
9+ if len(keys) == 1 and self._columns[keys[0]]._type == 'serialized':
10+ return result
11 for record_id in ids:
12 on_record_write.fire(session, self._name,
13- record_id, vals.keys())
14+ record_id, keys)
15 return result
16 orm.Model.write = write
17