openerp-expert-framework team mailing list archive

[Raphael Valyi <rvalyi@xxxxxxxxx>]

Hello Antony, my replies inline:

On Mon, Sep 26, 2011 at 8:06 PM, Antony Lesuisse <al@xxxxxxxxxxx> wrote:
>
> As usual the signal to noise ratio is low. So let me TLDR it: You want to
customize the business logic of openerp models more easily. OK.

I hope I'll prove you should read more carefully instead of being arrogant
with your community, read later.
>
>
> The mechanism we use in openerp is subclassing the model and overriding
methods (and we call super). Some existing methods should be split and we
will be happy to merge such kind refactoring.
>
> But, instead of that, what you propose is: not to refactor and to
introduce hooks everywhere and i really think this is insane. 'Towards a
cleaner a api' and linking your merge prop, to me, sounds like an oxymoron.

I hope I'll show you how wrong you are instead. And that's a pitty that you
seem so self assured and don't respect debating alternatives.
>
>
> example about the creation of a picking when needed:
>
> this is ok
>
https://code.launchpad.net/~openerp-dev/openobject-addons/trunk-sale-action-ship-create-al/+merge/77062
>
> this is not
>
https://code.launchpad.net/~akretion-team/openobject-addons/addons-sale-extensible-action-ship-create/+merge/76609

I'm sorry but I don't agree at all with your proposition, here is why:

So suppose we use your action_ship_create_picking in
https://code.launchpad.net/~openerp-dev/openobject-addons/trunk-sale-action-ship-create-al/+merge/77062
If I need to override the picking creation for a customization. A real
simple case fro he Brazilian localization: I need to propagate a m2o
category, say a "fiscal operation" to the picking.

How do I do that with your proposal?
So I override action_ship_create_picking. Then what? I see 2 options:

1) I call super to reuse your action_ship_create_picking:
then it sucks because your action_ship_create_picking already created the
record into the database! I will need to re-read it and re-write my field
into the database... If everybody does that as it is too often the case in
OpenERP addons, we end up doing 3x more requests than necessary to the
database, no wonder this is slow in the real life.
Not too say that may by I don't want to create a picking at all (in
base_project_costing I don't want) yet.

This may seems hacky but there are dozens of such real life situation where
OpenERP methods create a record we should delete then, very smart database
optimization indeed...
Let's take an other different concrete example to see where this reasoning
leads us: in Brazil delivery cost cannot be added as an order line, it needs
to be a global tax (eg deleted by overrider). The kind of code we have in
OpenERP is prety much like you suggest: it create the record first and
thinks later. I'm sorry to maintain need something smarter.

2) or I override action_ship_create_picking and I don't call super:
Suppose 2 modules do that: those modules are then incompatible unless you
create a third module to make the 3 compatibles (until you need one more
module), this is a very common situation with today's addons unfortunately.

Instead my solution suffer none of those pitfalls.
You said: "The mechanism we use in openerp is subclassing the model and
overriding methods (and we call super)."
Well read the merge, my solution follows this rule.
A bad non OOP hook would have been: value = self.get_value_from_hook(...)
And process value
This is not what I'm proposing, except for the key generation which I have
reasons for.

I'm sorry, but the only credible alternative that fulfills the requirements
(wise database usage and extensibility) is the proposal by Olivier to create
a 'prepare_picking' method that would prepare the data in the memory but
without hitting the database.

I hesitated between the two solutions and this is acceptable to me as well.
I'm not too sure if that's so much better, but I will try a new merge
proposal in that direction.
The reason I did what I did what I did instead of what Olivier proposed is
because I find it a reasonably "structuring" API. Instead, Olivier
suggestion is more of a woodstock/python solution where everyone get the
whole databag at the end of the method and can do what they want. To me It
means more freedom but also more risk of modules doing incompatible things
because of possible implicit and wrong assumptions they do over the databag
the receive from the overrider chain. Example: in my example I can partition
a picking over say different project phases. If I just get a databag,
chances are overriders wrongly assume implicitly there is only one picking.
So as you see, this is a profound debate with several possibilities with
strucurant and non structurant API's. I totally respect Olivier's poposal,
but I'm sorry I fail to accept your arrogance associated to your flawed
solution.

Here you introduce one method and only fix 50% the issue with the picking.
Specifically I also introduce one method for the picking and at least I meet
both the extensibility and database optimization requirements. So who is
wrong?

More on this: you said "introduce hooks everywhere and i really think this
is insane"
Yes there is a method explosion and this is a mess, and when we will
modularize the code the apparent mess of the core will increase (through
overall mess will decrease and this is my point).
But who is the culprit?
IMHO the culprit is not those asking for a more fine grained API to meet
their real life requirements where the sale module isn't enough (probably
80% of the guys that really work as OpenERP professionals). The culprit is
the lack of abstraction in OpenERP. Methods would better belong to specific
reusable organized scopes like modules (not sure how you do that in Python,
but I do it a lot in Ruby and it's clean; look at OOOR code if you want to
see some clean code from me).
I'm sorry, but the class model of OpenERP seems to fall down to wrapping
collection of records into a browse object that get their
method overrided as modules are loaded. Even the sale orders and invoices
product_id_change methods are not able to mutualize any common logic/code.
Tell me this is smart OOP...
I'm sorry but much better can be achieved. This is not because that is
better than average ERP systems that this is good code.

Now this is easy to put the guilt on the community. But this is normal,
third parties are focused to integration and 300 of us give you money
instead to take care of the framework. We don't have commit rights and
cannot afford working weeks without getting merges, so yes, we do more
superficial things in the core and real work elsewhere. You OpenERP SA are
instead expected to drive the debate about the non superficial things. You
can accuse me of communicating me too much, but sorry, doing the things
alone in the dark and sending the resulting invoice to your partners at the
end for code that don't meet their need is certainly not what we expect from
an open source project either.

Yes, as I said, we can also in the future fix those things by elegant AOP
kind of code. BUT you don't propose a better alternative and you also
propose to do nothing. Well, what I say is: let's face it, the world is not
perfect. Today, the real code that is being effectively deployed (eg not
just server and addons) is a real mess yes. But we need a solution to be
better than that. If perfection was an option I would take it too. You don'
propose perfect thing either at all. I propose action and convergence toward
better things release after release.
That being said, I will investigate Olivier's proposal.

I fear you guys don't understand our requirement because unlike us you don't
spend your time integrating OpenERP in the real life for countries other
than Belgium or France. Please take a step back and see how schizophrenical
this is too spend your time making a nice batch API orientation in the
virgin world of the server while real life addons spend their time
rebrowsing the same record one by one over and over and re-writting them
after customization.

Regards.


>
>
>
>
> On 09/26/2011 05:14 PM, Raphael Valyi wrote:
>>
>> Dear framework experts,
>>
>> like many others real life OpenERP integrators, at Akretion our average
>> OpenERP is with around 15 extra addons overriding natural behavior to
bring
>> value added features or localization.
>> So it's extremely important the normal addons modules can be overrided in
a
>> decent way.
>> Otherwise, features are not competitive to create, modules are not
compatible
>> between them (it's very serious, in Brazil module creating a
>> purchase/sale/invoice order line or picking should be overriden with a
>> localization module at this stage, imagine how hard it is for the new
comers,
>> many non trivial localizations are in the same situation...).
>> An other consequence is that the overall SQL activity is inefficient, no
>> matter how perfect the server layer is going to be.
>>
>> Let's take a concrete example: the sale module and the action_ship_create
>> method that generate the shipments and the procurements.
>> Today it's a massive single bloc of code and customization can only
happen
>> once the method has been executed (or before but then there is no reuse
and
>> compatibility at all). That means that we should re-search the created
items,
>> re-browse them. And depending how we change them later, other modules
doing
>> that are probably not compatible.
>>
>> *Instead, I propose to apply 2 general principles:*
>> *1) any method creating some important
>> business object  (specially something localizable) should give a chance
to a
>> potential module overrider to customize it BEFORE hitting the database
with
>> the create method. That creation method should provide all the creation
>> context (like the order, the product line for a shipment creation) so the
>> overrider can take that into account.*
>> *
>> *
>> *2) when there is a what I would call a "cardinalitity mismatch", that is
>> stock move lines belong to less (often 1) picking, account move lines
belong
>> to 1 move... Then there should be a key lookup in a hash to know if the
>> container object can be reused or need a new instance instead. The
generation
>> of hat key should be overridable in modules.*
>>
>>
>> *I proposed a little piece of such work that is waiting for a merge,
please
>> comment on here:*
>>
https://code.launchpad.net/~akretion-team/openobject-addons/addons-sale-extensible-action-ship-create/+merge/76609
>>
>>
>> In the future, that behavior could even be abstracted away in some
declarative
>> and customization fashion. However, let me warn against that. I suggest
to do
>> that in a declarative style only if perfect, else it's much worse than
python
>> code. Take the server actions for instance or the methods attached to
workflow
>> nodes. It's declarative but very little used (python methods are instead)
>> because it was far from perfect).
>>
>> And again, *let me advocate for a working solution right now for 6.1*.
Change
>> can be manage over the time. This has never been done with OpenERP, but
things
>> can be deprecated 6 months in advance as other framework do. We need a
>> solution to increase OpenERP adoption. Taking a concrete example, today
in
>> Brazil, considering that kind of catches less than 5 people are able to
put
>> OpenERP to work. This is not an ideal solution, neither for us, neither
for
>> OpenERP SA..
>> So I kindly request an evalution of this merge. If modules start using
that
>> modular API instead of hugly mono-bloc overrides, at least in the future
they
>> will have made a step toward a declarative refactoring, so this is not a
loss,
>> this is a win. I insist this refactoring is totally backward compatible
and
>> breaks no API, I was just warning here against the perfect being the
enemy of
>> the good that too often lead to inaction. The world is not perfect, third
>> parties contributions have limited resources and still we should converge
>> toward a better solution.
>>
>>
>> If this merge is accepted, then count on us to provide other such little
life
>> saver refactorings. This is no surprise that we have a divergence on the
>> relicense, well I propose to let this aside for now as we need a decent
6.1
>> release for the benefit of everybody. You will later be able to claim
that
>> those merges are under the dual license, at least now the dual license is
known.
>>
>>
>> Waiting for the merge evaluation.
>> (BTW we and many Brazilian partners and users are also waiting for a
merge of
>> l10n_br with correct chart of account more than 4 months now).
>
>
> _______________________________________________
> Mailing list: https://launchpad.net/~openerp-expert-framework
> Post to     : openerp-expert-framework@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~openerp-expert-framework
> More help   : https://help.launchpad.net/ListHelp


-- 
Raphaël Valyi
Founder and consultant
http://twitter.com/rvalyi
+55 21 2516 2954
www.akretion.com