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

Proposed by Sébastien BEAU - http://www.akretion.com
Status: Rejected
Rejected by: Olivier Dony (Odoo)
Proposed branch: lp:~sebastien.beau/openobject-server/autoload
Merge into: lp:openobject-server
Diff against target: 26 lines (+16/-0)
1 file modified
openerp/modules/loading.py (+16/-0)
To merge this branch: bzr merge lp:~sebastien.beau/openobject-server/autoload
Reviewer Review Type Date Requested Status
Olivier Dony (Odoo) Disapprove
OpenERP Framework Experts Pending
Review via email: mp+84512@code.launchpad.net

Description of the change

Hi
I propose a new feature for the server.
Indeed I have some module like "product_gift" that should do not have the same behaviour if an other module is installed in this case magentoerpconnect.
Indeed If magentoerpconnect is installed I want to load the mapping file automatically.
http://bazaar.launchpad.net/~openerp-commiter/openobject-addons/trunk-extra-addons/revision/5506.1.13

Yes I can create a third module that depend of magentoerpconnect and product_gift but If I chose this option I should create one module for magento, one for prestashop, one for spree one for... And this module will only have 3 line of code so not great solution.

Waht do you think?

Have a nice day

To post a comment you must log in.
Revision history for this message
Olivier Dony (Odoo) (odo-openerp) wrote :
Download full text (3.5 KiB)

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
...

Read more...

review: Disapprove
Revision history for this message
Nicolas Bessi - Camptocamp (nbessi-c2c-deactivatedaccount) wrote :

Hello,

I agree with you Olivier, but I do not find the current implementation of hiding module is that friendly for developers.

At that time you have added this domain on modules view action:
            <field name="domain">['!', ('category_id.parent_id','child_of','Hidden')]</field>

Instead of using a default filter on search view that is only accessible to Technical feature
 group members.

My 2 cent

Nicolas

Revision history for this message
Olivier Dony (Odoo) (odo-openerp) wrote :

On 12/14/2011 01:13 PM, Nicolas Bessi - Camptocamp wrote:
> Hello,
>
> I agree with you Olivier, but I do not find the current
> implementation of hiding module is that friendly for developers.

I agree that the implementation is incorrect at the moment, and that is
why I was not too specific yet about how to use these features ;-)
We're going to clean this up before RC1.

> At that time you have added this domain on modules view action:
> <field name="domain">['!',
> ('category_id.parent_id','child_of','Hidden')]</field>
>
> Instead of using a default filter on search view that is only
> accessible to Technical feature group members.

Yes I think that was a temporary attempt to quickly test out our new
module dashboard view (kanban) without the mess of technical modules,
but it needs improvements.
Your suggestion sounds good, we'll apply it while we cleanup these 2
areas: technical modules and auto-installing modules (I think the 2
concepts should be separated too)

Thanks for the feedback!

Revision history for this message
Raphaël Valyi - http://www.akretion.com (rvalyi) wrote :
Download full text (3.3 KiB)

On Wed, Dec 14, 2011 at 10:25 AM, Olivier Dony (OpenERP) <email address hidden>wrote:

> On 12/14/2011 01:13 PM, Nicolas Bessi - Camptocamp wrote:
> > Hello,
> >
> > I agree with you Olivier, but I do not find the current
> > implementation of hiding module is that friendly for developers.
>
> I agree that the implementation is incorrect at the moment, and that is
> why I was not too specific yet about how to use these features ;-)
> We're going to clean this up before RC1.
>

You know, if you did not called that previous pre-alpha in October a "RC1",
I'm sure we would have be been really less frightened a made less noise
about merges not being processed.

In any case, it seems a good readability improvement to distinguish modules
that have some concrete utility from purely abstract modules. If this is
what you are meaning with the new "Apps" flag, I think this will help quite
a lot.

Now, there are still important issues with module proliferation. Yesterday
I was discussing with Alexis to help porting the fleet maintenance module
to 6.1. Concrete issue we had: on the invoice we need a foreign key link
back to the main picking. So we added that many2one field in the module and
preyed methods would be modular enough so we could propagate the filed at
invoice creation also without killing performance with stored
fields re-computation.
Now imagine 2 modules need that kind of like (like report_intrastat may
need it too), then what do we do: each of those 2 modules add its own field
pointing to the same resource redundantly? Should they use the same key and
then assign it twice? Should we create yet a new base_foo abstract module
just to add that key. Probably it will be ignored by the community and many
such modules would exist... I mean those are the concrete issues we face
daily and for which no solution exists. It produce a module proliferation
hell that is totally opposed to out of the box usability which however
would be the only scalable model. So not sure Sebastien proposal was the
solution, but we certainly should adopt policies or features to deal with
those issues.
Experienced partners can always tune a particular installation to make it
work. But when I see your account managers that seem to be starving of new
implementations wondering why there are no more partners doing more
implementations at new frontiers, those issues that kill out of the box
compatibility certainly have an impact here.

>
> > At that time you have added this domain on modules view action:
> > <field name="domain">['!',
> > ('category_id.parent_id','child_of','Hidden')]</field>
> >
> > Instead of using a default filter on search view that is only
> > accessible to Technical feature group members.
>
> Yes I think that was a temporary attempt to quickly test out our new
> module dashboard view (kanban) without the mess of technical modules,
> but it needs improvements.
> Your suggestion sounds good, we'll apply it while we cleanup these 2
> areas: technical modules and auto-installing modules (I think the 2
> concepts should be separated too)
>
> Thanks for the feedback!
>
> --
>
> https://code.launchpad.net/~sebastien.beau/openobject-server/autoload/+merge/84512
> You...

Read more...

Revision history for this message
Sébastien BEAU - http://www.akretion.com (sebastien.beau) wrote :

Hi Olivier sorry for answering so late.
It's always a pleasure to read your comment. So after think twice, I agree with you, dependency should be explicite.
Also "auto-installation" feature seem a good idea for me (Openerp SA +1) and will solve many of my problem.

The only comment I can do about "auto-installation" feature will be to propose to donwload the "auto-install" module when we donwload a module linked on apps.
Indeed if someone success to download 2 modules without the third that manage the interaction fo the 2 modules. He will not have the behaviour expected.

Regarding my problem with mapping line I think you're right this should be in magentoerpconnect. I will improve my code to support inactive mapping.

Thanks again for your time.

Have a nice day

Unmerged revisions

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

[IMP] add autoload feature. If you add in your module a folder 'autoload' with a subfloder 'product' then if the module product is installed when you install or update your module the file in the subfolder will be loaded. you can see a sample of this in the module 'product_gift' in the extra-addons

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'openerp/modules/loading.py'
2--- openerp/modules/loading.py 2011-12-01 15:31:56 +0000
3+++ openerp/modules/loading.py 2011-12-05 17:52:27 +0000
4@@ -119,6 +119,22 @@
5 init mode.
6
7 """
8+ # If you create an "autoload" folder in your module, with subfolder the name of an other module.
9+ # OpenERP will automatically load the xml or csv file of this subfolder if the module is installed
10+ if kind == 'update_xml':
11+ mapping_dir = os.path.join(get_module_path(module_name), 'autoload')
12+ exist = os.path.exists(mapping_dir)
13+ if exist:
14+ for mapping_module in osutil.listdir(mapping_dir):
15+ cr.execute("select id from ir_module_module where name = '%s' and state in ('installed', 'to_update');"%mapping_module)
16+ res = cr.fetchall()
17+ if res:
18+ mapping_module_dir = os.path.join(mapping_dir, mapping_module)
19+ files = osutil.listdir(mapping_module_dir, recursive=True)
20+ for file in files:
21+ if file[-4:] in ['.csv', '.xml']:
22+ package.data['update_xml'].append('autoload/%s/%s'%(mapping_module,file))
23+
24 for filename in package.data[kind]:
25 logger.info("module %s: loading %s", module_name, filename)
26 _, ext = os.path.splitext(filename)