Merge lp:~openerp-community/openerp-web/trunk-smart-load-module into lp:openerp-web

Proposed by Simone Orsi
Status: Rejected
Rejected by: Xavier (Open ERP)
Proposed branch: lp:~openerp-community/openerp-web/trunk-smart-load-module
Merge into: lp:openerp-web
Diff against target: 67 lines (+39/-3)
1 file modified
addons/web/common/http.py (+39/-3)
To merge this branch: bzr merge lp:~openerp-community/openerp-web/trunk-smart-load-module
Reviewer Review Type Date Requested Status
Antony Lesuisse (OpenERP) Disapprove
Xavier (Open ERP) (community) Disapprove
Review via email: mp+89692@code.launchpad.net

Description of the change

improved load of modules by looking only for web addons** and by not failing silently when no 'static' folder is found.

** in general all the web module should respect the declaration of web=True in the manifest but since there seems to be a lof of missing declaration I check as a fallback for 'web0 in 'depends'.

To post a comment you must log in.
Revision history for this message
Xavier (Open ERP) (xmo-deactivatedaccount) wrote :

Sadly, the `web` key is a flag for 6.0 modules, its semantics are not defined in relations to openerp-web.

review: Disapprove
Revision history for this message
Simone Orsi (simone-orsi) wrote :

so chekcing only for static resource declared should be enough for handling this, isn it?

Revision history for this message
Simone Orsi (simone-orsi) wrote :

the problem here is that w/ the current implementation you have to define a static folder even if you don't need it at all. if you define a controller into your module but yoou don't put an *empty* static folder in it your module don't get loaded at all and silently. I think this is a bad behaviour.

Revision history for this message
Antony Lesuisse (OpenERP) (al-openerp) wrote :

I agree that the heuristic based on the /static/ dir is not be the best solution but, as we plan to change the module loading process in the server, i propose not to change this mechanism at the moment.

review: Disapprove
Revision history for this message
Simone Orsi (simone-orsi) wrote :

ok, I agree, but still I think you should log this at least in debug mode.
Moreover: any place where to see/read/test new module loading process?

Unmerged revisions

2029. By Simone Orsi

load module only if IS web one and don't fail silently if no 'static' folder is provided

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'addons/web/common/http.py'
2--- addons/web/common/http.py 2012-01-23 00:48:12 +0000
3+++ addons/web/common/http.py 2012-01-23 14:40:53 +0000
4@@ -502,6 +502,31 @@
5
6 return response(environ, start_response)
7
8+ def _is_web(self, manifest):
9+ """ given a module's manifest returns true if
10+ the module is recognized as 'web' module
11+ """
12+ dependencies = manifest.get('depends',[])
13+ depends_on_web = 'web' in dependencies
14+ # web modules should have always web=True
15+ # but sometimes is not, even for std web modules
16+ # ALSO 'web' module don't have both
17+ is_web = manifest.get('name')=='web'
18+ return manifest.get('web') or depends_on_web or is_web
19+
20+ def _need_static(self, manifest):
21+ """ given a manifest returns true if the module
22+ declares any static resource and needs a 'static' folder
23+ """
24+ needed = False
25+ keys = ('css','js','qweb')
26+ for k in keys:
27+ v = manifest.get(k)
28+ if isinstance(v,list) and len(v):
29+ needed = True
30+ break
31+ return needed
32+
33 def _load_addons(self, openerp_addons_namespace=True):
34 """
35 Loads all addons at the specified addons path, returns a mapping of
36@@ -512,9 +537,21 @@
37 for module in os.listdir(addons_path):
38 if module not in addons_module:
39 manifest_path = os.path.join(addons_path, module, '__openerp__.py')
40- path_static = os.path.join(addons_path, module, 'static')
41- if os.path.isfile(manifest_path) and os.path.isdir(path_static):
42+ if os.path.isfile(manifest_path):
43 manifest = ast.literal_eval(open(manifest_path).read())
44+ if not self._is_web(manifest):
45+ # if the module is not web skip it
46+ continue
47+ if self._need_static(manifest):
48+ path_static = os.path.join(addons_path, module, 'static')
49+ if not os.path.isdir(path_static):
50+ # if the module contains any static resource declaration
51+ # but does not provide any static directory skip it loudly
52+ msg = "'%s' declares static resources but '%s/static/' is missing. Skipping..."
53+ _logger.warning(msg % (module,module))
54+ continue
55+ else:
56+ statics['/%s/static' % module] = path_static
57 manifest['addons_path'] = addons_path
58 _logger.info("Loading %s", module)
59 if openerp_addons_namespace:
60@@ -523,7 +560,6 @@
61 m = __import__(module)
62 addons_module[module] = m
63 addons_manifest[module] = manifest
64- statics['/%s/static' % module] = path_static
65 for k, v in controllers_class.items():
66 if k not in controllers_object:
67 o = v()