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

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

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

Simone Orsi - Agile BG - Domsense
Simone Orsi - Agile BG - Domsense has proposed merging lp:~openerp-community/openerp-web/trunk-smart-load-module into lp:openerp-web.

Requested reviews:
  OpenERP R&D Web Team (openerp-dev-web)

For more details, see:
https://code.launchpad.net/~openerp-community/openerp-web/trunk-smart-load-module/+merge/89692

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'.
--
https://code.launchpad.net/~openerp-community/openerp-web/trunk-smart-load-module/+merge/89692
Your team OpenERP Community is subscribed to branch lp:~openerp-community/openerp-web/trunk-smart-load-module.

=== modified file 'addons/web/common/http.py'
--- addons/web/common/http.py 2012-01-23 00:48:12 +0000
+++ addons/web/common/http.py 2012-01-23 14:40:53 +0000
@@ -502,6 +502,31 @@
 
         return response(environ, start_response)
 
+    def _is_web(self, manifest):
+        """ given a module's manifest returns true if
+        the module is recognized as 'web' module
+        """
+        dependencies = manifest.get('depends',[])
+        depends_on_web = 'web' in dependencies
+        # web modules should have always web=True
+        # but sometimes is not, even for std web modules
+        # ALSO 'web' module don't have both
+        is_web = manifest.get('name')=='web'
+        return manifest.get('web') or depends_on_web or is_web
+
+    def _need_static(self, manifest):
+        """ given a manifest returns true if the module
+        declares any static resource and needs a 'static' folder
+        """
+        needed = False
+        keys = ('css','js','qweb')
+        for k in keys:
+            v = manifest.get(k)
+            if isinstance(v,list) and len(v):
+                needed = True
+                break
+        return needed
+
     def _load_addons(self, openerp_addons_namespace=True):
         """
         Loads all addons at the specified addons path, returns a mapping of
@@ -512,9 +537,21 @@
             for module in os.listdir(addons_path):
                 if module not in addons_module:
                     manifest_path = os.path.join(addons_path, module, '__openerp__.py')
-                    path_static = os.path.join(addons_path, module, 'static')
-                    if os.path.isfile(manifest_path) and os.path.isdir(path_static):
+                    if os.path.isfile(manifest_path):
                         manifest = ast.literal_eval(open(manifest_path).read())
+                        if not self._is_web(manifest):
+                            # if the module is not web skip it
+                            continue
+                        if self._need_static(manifest):
+                            path_static = os.path.join(addons_path, module, 'static')
+                            if not os.path.isdir(path_static):
+                                # if the module contains any static resource declaration
+                                # but does not provide any static directory skip it loudly
+                                msg = "'%s' declares static resources but '%s/static/' is missing. Skipping..."
+                                _logger.warning(msg % (module,module))
+                                continue
+                            else:
+                                statics['/%s/static' % module] = path_static
                         manifest['addons_path'] = addons_path
                         _logger.info("Loading %s", module)
                         if openerp_addons_namespace:
@@ -523,7 +560,6 @@
                             m = __import__(module)
                         addons_module[module] = m
                         addons_manifest[module] = manifest
-                        statics['/%s/static' % module] = path_static
         for k, v in controllers_class.items():
             if k not in controllers_object:
                 o = v()


_______________________________________________
Mailing list: https://launchpad.net/~openerp-community
Post to     : [hidden email]
Unsubscribe : https://launchpad.net/~openerp-community
More help   : https://help.launchpad.net/ListHelp
Reply | Threaded
Open this post in threaded view
|

Re: [Openerp-community] [Merge] lp:~openerp-community/openerp-web/trunk-smart-load-module into lp:openerp-web

Xavier (Open ERP)
Review: Disapprove

Sadly, the `web` key is a flag for 6.0 modules, its semantics are not defined in relations to openerp-web.
--
https://code.launchpad.net/~openerp-community/openerp-web/trunk-smart-load-module/+merge/89692
Your team OpenERP Community is subscribed to branch lp:~openerp-community/openerp-web/trunk-smart-load-module.

_______________________________________________
Mailing list: https://launchpad.net/~openerp-community
Post to     : [hidden email]
Unsubscribe : https://launchpad.net/~openerp-community
More help   : https://help.launchpad.net/ListHelp
Reply | Threaded
Open this post in threaded view
|

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

Xavier (Open ERP)
In reply to this post by Simone Orsi - Agile BG - Domsense
The proposal to merge lp:~openerp-community/openerp-web/trunk-smart-load-module into lp:openerp-web has been updated.

    Status: Needs review => Rejected

For more details, see:
https://code.launchpad.net/~openerp-community/openerp-web/trunk-smart-load-module/+merge/89692
--
https://code.launchpad.net/~openerp-community/openerp-web/trunk-smart-load-module/+merge/89692
Your team OpenERP Community is subscribed to branch lp:~openerp-community/openerp-web/trunk-smart-load-module.

_______________________________________________
Mailing list: https://launchpad.net/~openerp-community
Post to     : [hidden email]
Unsubscribe : https://launchpad.net/~openerp-community
More help   : https://help.launchpad.net/ListHelp
Reply | Threaded
Open this post in threaded view
|

Re: [Openerp-community] [Merge] lp:~openerp-community/openerp-web/trunk-smart-load-module into lp:openerp-web

Simone Orsi - Agile BG - Domsense
In reply to this post by Simone Orsi - Agile BG - Domsense
so chekcing only for static resource declared should be enough for handling this, isn it?
--
https://code.launchpad.net/~openerp-community/openerp-web/trunk-smart-load-module/+merge/89692
Your team OpenERP Community is subscribed to branch lp:~openerp-community/openerp-web/trunk-smart-load-module.

_______________________________________________
Mailing list: https://launchpad.net/~openerp-community
Post to     : [hidden email]
Unsubscribe : https://launchpad.net/~openerp-community
More help   : https://help.launchpad.net/ListHelp
Reply | Threaded
Open this post in threaded view
|

Re: [Openerp-community] [Merge] lp:~openerp-community/openerp-web/trunk-smart-load-module into lp:openerp-web

Simone Orsi - Agile BG - Domsense
In reply to this post by Simone Orsi - Agile BG - Domsense
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.
--
https://code.launchpad.net/~openerp-community/openerp-web/trunk-smart-load-module/+merge/89692
Your team OpenERP Community is subscribed to branch lp:~openerp-community/openerp-web/trunk-smart-load-module.

_______________________________________________
Mailing list: https://launchpad.net/~openerp-community
Post to     : [hidden email]
Unsubscribe : https://launchpad.net/~openerp-community
More help   : https://help.launchpad.net/ListHelp
Reply | Threaded
Open this post in threaded view
|

Re: [Openerp-community] [Merge] lp:~openerp-community/openerp-web/trunk-smart-load-module into lp:openerp-web

Antony Lesuisse (OpenERP)
In reply to this post by Simone Orsi - Agile BG - Domsense
Review: Disapprove

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.
--
https://code.launchpad.net/~openerp-community/openerp-web/trunk-smart-load-module/+merge/89692
Your team OpenERP Community is subscribed to branch lp:~openerp-community/openerp-web/trunk-smart-load-module.

_______________________________________________
Mailing list: https://launchpad.net/~openerp-community
Post to     : [hidden email]
Unsubscribe : https://launchpad.net/~openerp-community
More help   : https://help.launchpad.net/ListHelp
Reply | Threaded
Open this post in threaded view
|

Re: [Openerp-community] [Merge] lp:~openerp-community/openerp-web/trunk-smart-load-module into lp:openerp-web

Simone Orsi - Agile BG - Domsense
In reply to this post by Simone Orsi - Agile BG - Domsense
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?
--
https://code.launchpad.net/~openerp-community/openerp-web/trunk-smart-load-module/+merge/89692
Your team OpenERP Community is subscribed to branch lp:~openerp-community/openerp-web/trunk-smart-load-module.

_______________________________________________
Mailing list: https://launchpad.net/~openerp-community
Post to     : [hidden email]
Unsubscribe : https://launchpad.net/~openerp-community
More help   : https://help.launchpad.net/ListHelp