[17.0][ADD] base_write_diff: new module

[SilvioC2C]

No description provided.

[legalsylvain]

interesting feature.

Could you share statistics on performance improvements, if you have any.

[SilvioC2C]

@legalsylvain hello 👋

To be honest, I don't have stats about performance diffs between the usage of write_diff() against write().

However, the features included in this module were before included in a custom module we've been using in a production environment (for a few months now) within this context:

• Odoo needs to receive data from an external service, and the external service always sends all fields' values to Odoo
• when importing these values, even if the new values are exactly the same as the old ones, Odoo would trigger lots of recomputations (eg: when sending data about SO lines' amounts), and lots of registry cache invalidations would occur (see the product.product.write() override in USAGE.md)
• with the features now included in this module, we were able to prevent all that from happening and improve the overall performances of each import execution

[yankinmax]

Thanks @SilvioC2C , great work!

[ivantodorovich]

AFAIK post_load is executed globally (meant for server wide modules) and so in a multi-tenant deployment it would affect databases without this module installed.

AFAICS this is only needed for the context-based usage. TBH I'd drop it and keep only the explicit write_diff method usage (KISS)

Alternatively, if you want to keep it, I'd explore _register_hook / _unregister_hook. This is the mechanism used in the core base_automation module

[SilvioC2C]

That makes sense, thanks! I'll update the module accordingly.

[ivantodorovich]

This will not respect other modules doing the same thing, cause the original_write is evaluated at import time.

For example, this probably breaks the base_automation module hooks

You need to follow the pattern used in base_automation, that builds the patch method on the fly and uses write.origin(..) to call the original method

[SilvioC2C]

@ivantodorovich mind taking another look? It should be fine now

[ivantodorovich]

IMO it's still wrong. In a multi-tenant deployment you'd have a race condition with several registries setting/unsetting _origin.

We need to patch the registry's loaded model, not the BaseModel definition itself.

For that the method needs to be created on-the-fly, and patched like this: https://github.com/odoo/odoo/blob/69828835c926485ec80ed92e14dd11fbc6c1caaa/addons/base_automation/models/base_automation.py#L786-L811

[ivantodorovich]

TBH I'm not even sure you need this, compared to a regular write override 🤔
With _register_hook you're override would be the first to execute, cause the method would be added to the top of the MRO
With a regular write override on base you're override would likely be the last one to execute (except other modules also try this, then it depends on their loading order)

I thought you wanted the first scenario, cause that would make sense logically for me. ..but maybe I'm wrong, and you wanted the second one ?
In this case a simple override would be fine -- cause it's very very unlikely other modules try to override the base model write method to ADD new values.

[SilvioC2C]

@ivantodorovich you're right, a write() override was enough, I've updated the code accordingly

[OCA-git-bot]

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖