Merge lp:~akretion-team/openobject-server/trunk-extra-depdencies-info into lp:openobject-server

Proposed by Raphaël Valyi - http://www.akretion.com
Status: Rejected
Rejected by: Vo Minh Thu
Proposed branch: lp:~akretion-team/openobject-server/trunk-extra-depdencies-info
Merge into: lp:openobject-server
Diff against target: 47 lines (+5/-3)
3 files modified
openerp/addons/base/module/module.py (+2/-0)
openerp/modules/db.py (+1/-1)
openerp/modules/graph.py (+2/-2)
To merge this branch: bzr merge lp:~akretion-team/openobject-server/trunk-extra-depdencies-info
Reviewer Review Type Date Requested Status
Vo Minh Thu (community) Disapprove
Review via email: mp+114172@code.launchpad.net

Description of the change

DESCRIPTION:
 __openerp__.py "depends" module meta information list can now contain a dictionary instead of just the string name, in the case of such a dictionary, the 'name' value of the dictionary will be used instead of the string name. The idea is that we can now use that dictionary to store extra information about the dependencies, like the URL of the repository where a depedency should be downloaded, specially if it's not a "core" addons. This will help the "extra-addons" monolithic branch decommissioning and point the developpers or the installation tools to the proper dependencies versions.

EXAMPLE:
previously we could have the magenerpconnect __openerp__.py module like:
{
    "name" : "Magento e-commerce",
    "version" : "1.0",
    "depends" : ["base",
                 "product",
                 "product_m2mcategories",
                 'delivery',
                 "base_sale_multichannels",
                 "product_images_olbs",
                 "product_links",
                ],
...

Now it could point to the proper modules versions like:
{
    "name" : "Magento e-commerce",
    "version" : "1.0",
    "depends" : ["base",
                 "product",
                 {"name": "product_m2mcategories", "bzr": "lp:product-extra-addons/openerp6.1-module"},
                 'delivery',
                 {"name": "base_sale_multichannels", "bzr": "lp:e-commerce-addons/openerp6.1-module",
                 {"name": "product_images_olbs", "bzr": "lp:product-extra-addons/openerp6.1-module"},
                 {"name": "product_links", "bzr": "lp:e-commerce-addons/openerp6.1-module"},
                ],
...

RATIONALE:
Due to the open source nature of the OpenERP ecosystem with many small professionals innovating constantly for small businesses over short period of time, during a single 6 months/1 year OpenERP cycle, several version of a third party module can be made. An example was the magentoerpconnect version for which we had a stable 6.1 version and then a new experimental version that has finally become the new stable version at the end of the 6.1 cycle.
Previously we had a big problem for a module to specify its dependencies because we had no way to specify a given repository or version among the several possible versions existing for the given OpenERP release.
Now, we can decide to normalize some specific keys of the new dictionary to specify a repository or a version or even a minimal revision to be used.
In that first iteration, the idea is simply to use that information manually when doing installation of complex modules. But of course, in the future, installation tools (existing ones or new ones) could use it too to provide a one click installation of any complying third party modules.

I did this taking inspiration on how a package manager like Bundler http://gembundler.com/ can install git modules dependencies as explained here in GEMS/GIT http://gembundler.com/man/gemfile.5.html
By the way it seems that Python PIP/egg tool is currently not up to the mark as it seems that it cannot fetch Python eggs from code repositories such as git or bzr but can only download dependencies from specific location as mentioned here http://peak.telecommunity.com/DevCenter/setuptools#declaring-dependencies
But again, this is a detail, we are here just making the file format compatible for possible future tooling but we are not implementing any such tooling yet. Third parties could do that as they wish once at least they have the format.

To post a comment you must log in.
4237. By Akretion Bot <email address hidden>

[REF] simplification of previous commit allowing dictionaries to describe dependencies; also better if else statements

Revision history for this message
Vo Minh Thu (thu) wrote :

After much internal discussion about this merge prop. we have decided it was not the way to go.

The merge prop. is pretty simple: make the depends entry a list of dicts instead of a list of strings. But still module versioning and meta-data can become a complicated matter rapidly.

If really needed for your own purpose, I guess you might add another, similarly named entry in the __openerp__.py file.

From a purely technical perspective, your patch would be simpler by having the function `load_information_from_description_file` in openerp.modules.module return the dicts instead of the strings so the representation is always the same.

A more lenghty view on the matter is given by this merge prop. on the doc: https://code.launchpad.net/~openerp-dev/openobject-server/trunk-module-versioning-vmt/+merge/128668.

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

Hello Vo,

First thank you for your analysis. Unfortunately, I should say you are here defending a naive vision of modularity in OpenERP, it doesn't match the experience we integrators have from doing real OpenERP (not just framework) projects.

I'll give you a few examples where your system will never work:

1) suppose we decomission the extra addons and start moving addons to individual branches of the 7.0 serie. 90% of those module will be broken at the beginning (because of changes in OpenERP 7.0 or because they were already broken). Does it mean they should be flagged as the stable official versions? You propose what? create new ones with new names? That will create an ir_model_data hell and leave the 7.0 polluted by those original broken modules that will accumulate over and over.

2) look at complex modules like the ecommerce connectors or complex localizations, they use to evolve and accommodate to the OpenERP changes over the versions. They never fit into that simplistic vision. For instance OpenERP SA hired 10 partners here in Brazil. I'm not sure that this "we don't care about your case" message will match their expectation to lower the entry barrier to be able to install their localisation and eventually forward money to OpenERP SA. Notice that I'm never here speaking just for me, as skilled guys like us are perfectly able to manage that dependency hell, but really I've been advocating for your own business model where you try to sell services through a partners network.

I agree that dependency hell exists. But guess what: there are dozen of existing package managers that already fixed the problem in the best possible way. Imagine that Debian or Ubuntu think the way OpenERP think and discard dependency management, where would they be today? Or do you think an ERP is actually so much simpler than a Linux distro that it can afford avoiding the dependency question?
The dependency hell already exists, what you are doing is you are ignoring it instead of addressing it at least in some minimal ways.

Additionally, OpenERP SA speaks as if OpenERP were stable. Hell no! just see how much modules have become incompatible with that new 7.0 version to understand it's absolutely not the case. And will it all the sudden become stable at God blessed version 7.0? Absolutely not! the new on_change API that should have largely fixed the main modularity issues in OpenERP have been dropped from 7.0 and will be left for latter. So when you will in 1 year or may be more finally implement it, modules will all need to evolve again to stop being incompatible together...

And sorry to say it, but the world clock doesn't revolve around OpenERP SA un-predictible semi hidden possibly yearly ""stable"" release. No contributors take their funding from their implementation projects that follow their own schedule (faster than a year) meaning that modules evolve at any time, during the OpenERP ""stable"" lifecycle. That's why successful open source usually makes the most of dependency management to enable that evolution and multi-scale life cycles of software components.

I'm very disappointed by this position. Of course, we will add the key we need in the __ope...

Read more...

Unmerged revisions

4237. By Akretion Bot <email address hidden>

[REF] simplification of previous commit allowing dictionaries to describe dependencies; also better if else statements

4236. By Akretion Bot <email address hidden>

[IMP] __openerp__.py "depends" module meta information list can now contain a dictionary instead of just the string name,
in the case of such a dictionary, the 'name' value of the dictionary will be used instead of the string name.
The idea is that we can now use that dictionary to store extra information about the dependencies,
like the URL of the repository where a depedency should be downloaded, specially if it's not a "core" addons.
This will help the "extra-addons" monolithic branch decommissioning and point the developpers or the installation tools
to the proper dependencies versions.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'openerp/addons/base/module/module.py'
2--- openerp/addons/base/module/module.py 2012-06-19 15:16:26 +0000
3+++ openerp/addons/base/module/module.py 2012-07-10 14:10:25 +0000
4@@ -561,6 +561,8 @@
5 def _update_dependencies(self, cr, uid, mod_browse, depends=None):
6 if depends is None:
7 depends = []
8+ else:
9+ depends = [i['name'] if isinstance(i, dict) else i for i in depends]
10 existing = set(x.name for x in mod_browse.dependencies_id)
11 needed = set(depends)
12 for dep in (needed - existing):
13
14=== modified file 'openerp/modules/db.py'
15--- openerp/modules/db.py 2012-06-11 10:36:53 +0000
16+++ openerp/modules/db.py 2012-07-10 14:10:25 +0000
17@@ -94,7 +94,7 @@
18 dependencies = info['depends']
19 for d in dependencies:
20 cr.execute('INSERT INTO ir_module_module_dependency \
21- (module_id,name) VALUES (%s, %s)', (id, d))
22+ (module_id,name) VALUES (%s, %s)', (id, d['name'] if isinstance(d, dict) else d))
23 cr.commit()
24
25 def create_categories(cr, categories):
26
27=== modified file 'openerp/modules/graph.py'
28--- openerp/modules/graph.py 2012-01-24 12:42:52 +0000
29+++ openerp/modules/graph.py 2012-07-10 14:10:25 +0000
30@@ -59,7 +59,7 @@
31
32 def add_node(self, name, info):
33 max_depth, father = 0, None
34- for n in [Node(x, self, None) for x in info['depends']]:
35+ for n in [Node(x['name'] if isinstance(x, dict) else x, self, None) for x in info['depends']]:
36 if n.depth >= max_depth:
37 father = n
38 max_depth = n.depth
39@@ -110,7 +110,7 @@
40
41 while packages and current > later:
42 package, info = packages[0]
43- deps = info['depends']
44+ deps = [i['name'] if isinstance(i, dict) else i for i in info['depends']]
45
46 # if all dependencies of 'package' are already in the graph, add 'package' in the graph
47 if reduce(lambda x, y: x and y in self, deps, True):