Contributors mailing list archives

contributors@odoo-community.org

Browse archives

Avatar

Re: Overwriting a method in OCA module.

by
Acsone SA/NV, Laurent Mignon
- 15/09/2015 09:49:25
You should add a caution defining when 'monkey patching' is allowed and what are the risks....
IMO we should also use a dedicated logger to keep a trace of applied patches when the server starts.

On Tue, Sep 15, 2015 at 9:38 AM, Kitti U. <kittiu@ecosoft.co.th> wrote:
Hi Stefan,

Thank you for your clarification. In a way I think MonkeyPatch is still useful. Please confirm if my understanding is correct.


Here is what I want to propose for contributing.md:

Monkey Patch is simply,
  • The class that will Patch (replace) directly to the original module A's method. (it is equal to modifying method in module A directly, and still preserve the calling chain)
Let's use your scenario, 
  • Module A as original module
  • Module B inherit module A (by calling super())
  • Module C (our new module), which also want to inherit Module A (but face overwrite challenge)
What developer should do
====================

1. In module C, create a MonkeyPatch to module A with a new "hook_point". (just the hook point only). -> as the result we can think of module A as if it already have the hook_point.

i.e., / Module C / account_voucher_monkeypatch.py

2. In module C, we write method as if hook_point exists (now we can call super())

i.e., / Module C / account_voucher.py

3. Submit new PR to Odoo, proposing the hook_point to original file in module A (same as what we have done in Monkey Patch)

4. As soon as Odoo merge the PR, we can get the latest code and delete the monkey patch from module C.

Monkey Patch can have some benefits
==============================
  • MonkeyPatch will protect module B from being out of chain
  • MonkeyPatch file will serve as warning message that module is being overwritten, and PR must be raised and fixed.
  • MonkeyPatch can be easily removed, once the PR is accepted, as hook_point is incoperated into the core.

What do you guys think?
Kitti

On Tue, Sep 15, 2015 at 12:37 PM, Stefan Rijnhart <stefan@opener.am> wrote:
On 13-09-15 10:37, Kitti U. wrote:
> Dear Stefan,
>
> Thank you for brought up an interesting topic. Still I do not quite
> understand it.
>
> As I try to understand from the code, the MonkeyPatch class (i.e.,
> account.voucher.monkeypatch), its duty is to redirect the original
> method to our own modified method. Right?
>
> Though it will help not having to overwrite the method directly to the
> underlining Class (i..e, account.voucher), but the affect is the same
> as a plain method overwrite. Is it not?
>
> For example, months later, the original method has been enhanced by
> Odoo S.A (core code). Those changes will be ignored, as it the
> monkeypatch class will still help redirect it to our modified method
> (which now become outdated).
>
> So, this won't help, is it? What will be the benefit of using it when
> compare to overwriting it?

Hi Kitti,

consider the case where module B overrides the method from module A.
Your module C overwrites the method, and as it happens, is loaded in
Odoo after module B. Your version does not call super(), which breaks
the inheritance of module A from module B, as the override from B is
never called.

When applying a monkeypatch instead of overwriting the method from your
module C, then the inheritance chain in which B calls the method from
module A remains intact. B's override is still registered in the ORM as
the first entry point of the method and when B calls super(), it ends up
calling the monkeypatched method from your module C.

You are right in that both methods can cause bugfixes in the original
version of the method to be easily overlooked.

For the record, the class in which the monkeypatch is applied is pretty
much irrelevant.

Regards,
Stefan.


_______________________________________________
Mailing-List: http://odoo-community.org/groups/oca-contributors-15
Post to: mailto:contributors@odoo-community.org
Unsubscribe: http://odoo-community.org/groups?unsubscribe




--
Mobile: +66-(0)8-1841-7480
Your ERP Partner => www.ecosoft.co.th

_______________________________________________
Mailing-List: http://odoo-community.org/groups/oca-contributors-15
Post to: mailto:contributors@odoo-community.org
Unsubscribe: http://odoo-community.org/groups?unsubscribe




--
Laurent Mignon
Senior software engineer

Tel: +352 20 21 10 20 32
Fax: +352 20 21 10 21
Email: laurent.mignon@acsone.eu

Acsone SA, Succursale de Luxembourg 
22, Zone Industrielle
​ ​
​| L-8287 Kehlen, Luxembourg
TVA LU24733605
​ | ​RCS B160400​

Acsone sa/nv
Boulevard de la Woluwe 56, b4  Woluwedal
​ | B-1200 Bruxelles - Brussel 
RPM Bruxelles 0835.207.216 RPR Brussel  



Reference