Open Stack

On 20 June 2015 at 09:11, Thai Q Tran <tqtran at us.ibm.com> wrote:

> -----Richard Jones <r1chardj0n3s at gmail.com> wrote: -----
> To: "OpenStack Development Mailing List (not for usage questions)" <
> openstack-dev at lists.openstack.org>
> From: Richard Jones <r1chardj0n3s at 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.
> https://review.openstack.org/#/c/193681/
>

Wow, that's a lot of shuffling of deckchairs! I agree with the approach,
but it seems like the only thing we're going to get done in Liberty is move
a bunch of files around :/


      Richard
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20150622/2ba89863/attachment.html>