Re: [openstack-dev] [horizon] Angular dependencies and the hz.dashboard namespace
On 20 June 2015 at 09:11, Thai Q Tran tqt...@us.ibm.com wrote: -Richard Jones r1chardj0...@gmail.com wrote: - To: OpenStack Development Mailing List (not for usage questions) openstack-dev@lists.openstack.org From: Richard Jones r1chardj0...@gmail.com Date: 06/19/2015 03:49PM Subject: [openstack-dev] Angular dependencies and the hz.dashboard namespace The Adding Angular Identity Dashboard[1] patch exposed an issue I saw previously that worried me[2]. I believe during recent Horizon work the concept of angular module namespaces and dependencies have been conflated. There's this idea that if you create a submodule inside a module namespace you *must* have that module depend on that submodule. I believe that is incorrect - just look at the angular codebase itself, and how it is used. If you want the ngCookies module in a couple of places then you have those modules depend on ngCookies (or, more likely, you just add it as a dependency to the app). The point is it's not just added automatically to the ng module as a dependency. If you need to use a module's functionality, you depend on the module. You don't just have all your modules *automatically* pulled in as dependencies. I have proposed a patch to remedy this for the existing optional[3] project dashboard[4]. I believe it is unnecessary to add extension of the hz.dashboard dependency list[5] since we already have an extensible dependency list for the application itself (which the hz.dashboard dependency list just extends). To answer the question explicitly raised what is the point of having the hz.dashboard module if it has no dependencies? It does two things: it defines the namespace in a formal manner (by itself unnecessary, but it's still nice to do) and it defines a constant which is used by other code. Eventually it may define more. There is an important difference between Python modules and Angular modules here - using a Python module like hz.dashboard in this way could cause problems because of the way Python sub-module namespaces and import ordering work. Angular's modules work very differently, and are not burdened by the same issues. In Python that constant would most likely have to be pushed out to a sub-module to avoid import loops. Richard [1] https://review.openstack.org/190852 [2] https://bugs.launchpad.net/horizon/+bug/1466713 [3] There may be other issues that prevent the project dashboard being optional - there are dependencies in Python-land from the admin dashboard hard-coded over to the project dashboard, for example. It might be a good idea to remedy that, since I think it probably exposes some other structural issues in the plugin mechanism we're providing to deployers. [4] https://review.openstack.org/#/c/193401/ [5] https://review.openstack.org/193671 1. Need to retain the same file structure so that pluggins continue to work. Example: https://github.com/stackforge/monasca-ui/tree/master/monitoring Basically we have existing pluggins that use this file structure, we need to honor it. This is not directly related to what we are talking about, but it does mean that we need to move static files out of /openstack_dashboard/static/MYDASHBOARD/* and into /openstack_dashboard/dashboards/MYDASHBOARD/static/* Yep! Auto-discovery of static resources will need to also honor the pluggin model above, hence the file structure above. You will still need to manually define the ADD_ANGULAR_MODULES in your enabled file, auto-discovery doesn't know what you want enabled. Sean's patch is going to do that, but having some issues with SCSS. https://review.openstack.org/#/c/191592/ As far as I can tell that patch doesn't alter the current ADD_ANGULAR_MODULES behaviour? hz.dashboard module will be empty because the hz.dashboard.MYDASHBOARD module will live at the app level via ADD_ANGULAR_MODULES. I would argue that it makes no sense to have an empty module, my preference is to just delete it. Constants are globally available in the app, something I think actually should be avoided, not encouraged. OK, seems reasonable. Having hz.dashboard.tech-debt and workflow in the enabled file is not correct. They are core components needed by all dashboards and should be loaded by default, not via the pluggin mechanism. https://review.openstack.org/#/c/193401/4/openstack_dashboard/enabled/_10_project.py Lets say I have my own dashboard call MYDASHBOARD, and I decided to disable all other dashboards except mine, all of a sudden, things will break horribly because tech-debt and workflow are not loaded. I would have to either: a. load the _10_project enabled file b. copy/paste over the dependencies from _10_project Furthermore, if I have hz.dashboard module, where do I load that, in _10_project or _x_MYDASHBOARD? Same issue when I disable entire dashboards. I completely agree! Tyr's patch will address this problem by having a core module.
Re: [openstack-dev] [horizon] Angular dependencies and the hz.dashboard namespace
Heres a summary of what we talked about:1.Need to retain the same file structure so that pluggins continue to work.Example:https://github.com/stackforge/monasca-ui/tree/master/monitoringBasically we have existing pluggins that use this file structure, we need to honor it.This is not directly related to what we are talking about, but it does mean that we need to move static files out of/openstack_dashboard/static/MYDASHBOARD/* and into/openstack_dashboard/dashboards/MYDASHBOARD/static/*2.Auto-discovery of static resources will need to also honor the pluggin model above, hence the file structure above.You will still need to manually define the ADD_ANGULAR_MODULES in your enabled file, auto-discovery doesn't know what you want enabled.Sean's patch is going to do that, but having some issues with SCSS.https://review.openstack.org/#/c/191592/3.hz.dashboard module will be empty because the hz.dashboard.MYDASHBOARD module will live at the app level viaADD_ANGULAR_MODULES. I would argue that it makes no sense to have an empty module, my preference is to just delete it.Constants are globally available in the app, something I think actually should be avoided, not encouraged.4.Having hz.dashboard.tech-debt and workflow in the enabled file is not correct.They are core components needed by all dashboards and should be loaded by default, not via the pluggin mechanism.https://review.openstack.org/#/c/193401/4/openstack_dashboard/enabled/_10_project.pyLets say I have my own dashboard call MYDASHBOARD, and I decided to disable all other dashboards except mine,all of a sudden, things will break horribly because tech-debt and workflow are not loaded. I would have to either:a. load the _10_project enabled fileb. copy/paste over the dependencies from _10_projectFurthermore, if I have hz.dashboard module, where do I load that, in _10_project or _x_MYDASHBOARD?Same issue when I disable entire dashboards.Tyr's patch will address this problem by having a core module.https://review.openstack.org/#/c/193681/-Richard Jones r1chardj0...@gmail.com wrote: -To: "OpenStack Development Mailing List (not for usage questions)" openstack-dev@lists.openstack.orgFrom: Richard Jones r1chardj0...@gmail.comDate: 06/19/2015 03:49PMSubject: [openstack-dev] Angular dependencies and the hz.dashboard namespaceThe "Adding Angular Identity Dashboard"[1] patch exposed an issue I saw previously that worried me[2].I believe during recent Horizon work the concept of angular module namespaces and dependencies have been conflated. There's this idea that if you create a submodule inside a module namespace you *must* have that module depend on that submodule. I believe that is incorrect - just look at the angular codebase itself, and how it is used. If you want the ngCookies module in a couple of places then you have those modules depend on ngCookies (or, more likely, you just add it as a dependency to the app). The point is it's not just added automatically to the "ng" module as a dependency. If you need to use a module's functionality, you depend on the module. You don't just have all your modules *automatically* pulled in as dependencies. I have proposed a patch to remedy this for the existing "optional"[3] project dashboard[4].I believe it is unnecessary to add extension of the hz.dashboard dependency list[5] since we already have an extensible dependency list for the application itself (which the hz.dashboard dependency list just extends).To answer the question explicitly raised "what is the point of having the hz.dashboard module if it has no dependencies?" It does two things: it defines the namespace in a formal manner (by itself unnecessary, but it's still nice to do) and it defines a constant which is used by other code. Eventually it may define more. There is an important difference between Python modules and Angular modules here - using a Python module like hz.dashboard in this way could cause problems because of the way Python sub-module namespaces and import ordering work. Angular's modules work very differently, and are not burdened by the same issues. In Python that constant would most likely have to be pushed out to a sub-module to avoid import loops. Richard[1]https://review.openstack.org/190852[2]https://bugs.launchpad.net/horizon/+bug/1466713[3] There may be other issues that prevent the project dashboard being optional - there are dependencies in Python-land from the admin dashboard hard-coded over to the project dashboard, for example. It might be a good idea to remedy that, since I think it probably exposes some other structural issues in the plugin mechanism we're providing to deployers.[4]https://review.openstack.org/#/c/193401/[5]https://review.openstack.org/193671 __OpenStack Development Mailing List (not for usage questions)Unsubscribe:
