← Back to team overview

credativ team mailing list archive

Re: [Merge] lp:~sebastien.beau/openobject-server/autoload into lp:openobject-server

 

Review: Disapprove

This idea might look appealing on a small scale, but I think it means a lot of trouble in the long run, and there are perhaps better alternatives to solve your problem.

1.- Why this is dangerous?
Doing this you introduce a hidden dependency mechanism, and you are exposing the developer to a lot of future headaches. The database loading mechanism is more complicated than meets the eye, and it is easy to introduce unpredictability in it. It's not only at installation time, but at any time the database/instance is loaded. The system computes the dependency graph and then loads modules in dependency layers, but the models and the data of modules are not loaded at the same time. With a hidden dependency mechanism, your "hidden directory" might be loaded at a moment where the dependent module is not initialized at all, or only partially initialized. In both cases you will get unpredictable results. You will also want to have both code and data in this way, and more problems will arise because they would be handled out of the normal flow.
If you don't want all these problem to occur, then the system has to treat these hidden dependencies as real dependencies, and you're making the whole thing even more complicated than it is, while still hiding the dependency to unwary eyes. And all this trouble for what benefit?

It's also been argued that this would make module dependency management easier. I believe that introducing such a mechanism will in fact make it much worse in the long run, because you will get a lot of unexpected side effects as developers start using these hidden dependencies. If we want to help with dependency management we should make the whole think simpler and more obvious, rather than introduce tricky/hidden behaviors in there, don't you think?


2.- What alternatives?
Having to make a small module to make the connection between 2 otherwise unrelated ones seems okay to me as a general principle. It's better to have it explicit than implicit. And if it looks like a lot of trouble to you, apparently that's for the best because it makes you think twice about creating them :-) Perhaps your design can be improved to avoid their needs, or perhaps it's actually good to have these explicit modules?
I did not check your example thoroughly, but maybe the external mappings you need for wiring "product_gift" with the various eCommerce modules could simply be pre-installed with these modules, and inactive in some fashion until they're actually used by the corresponding models on the OpenERP side. You would think that providing a generic set of mappings for e.g. Magento would be the responsibility of the magento module, and not that of the various unrelated modules like product_gift, wouldn't you?

Finally, the standard addons have many such technical modules (project_mrp, project_timesheet, sale_crm, and even more in 6.1). And even though we're happy with that on a design point of view, we agree they shouldn't be exposed to the users, as they only add noise to the list of "features". So we're also adding two small mechanisms to make this more user-friendly in v6.1:
- A special module category for hiding "technical" modules like this by default
- A new mechanism for toggling the auto-installation of a module as soon as all its dependencies are installed. This way you won't have to manually install "sale_crm", as soon as you install "crm" and "sale" it will be automatically installed.

Perhaps these various things would provide viable and cleaner alternatives for your needs?
-- 
https://code.launchpad.net/~sebastien.beau/openobject-server/autoload/+merge/84512
Your team OpenERP Framework Experts is requested to review the proposed merge of lp:~sebastien.beau/openobject-server/autoload into lp:openobject-server.