Re: [openstack-dev] [horizon] Angular panel enable/disable not overridable in local_settings

2016-08-03 Thread Rob Cresswell
Ah, I see. I think I can understand why that was done initially, but since 
using UPDATE_HORIZON_CONFIG is already going beyond the scope of the enabled 
file since its directly affecting an exposed setting, I think we should just 
use the setting (and document it appropriately in the settings.rst)

Rob

On 2 August 2016 at 23:39, Richard Jones 
> wrote:
On 3 August 2016 at 00:32, Rob Cresswell 
> wrote:
Hi all,

So we seem to be adopting a pattern of using UPDATE_HORIZON_CONFIG in the 
enabled files to add a legacy/angular toggle to the settings. I don't like 
this, because in settings.py the enabled files are processed *after* 
local_settings.py imports, meaning the angular panel will always be enabled, 
and would require a local/enabled file change to disable it.

My suggestion would be:

- Remove current UPDATE_HORIZON_CONFIG change in the swift panel and images 
panel patch
- Add equivalents ('angular') to the settings.py HORIZON_CONFIG dict, and then 
the 'legacy' version to the test settings.

I think that should run UTs as expected, and allow the legacy/angular panel to 
be toggled via local_settings.

Was there a reason we chose to use UPDATE_HORIZON_CONFIG, rather than just 
updating the dict in settings.py? I couldn't recall a reason, and the original 
patch ( https://review.openstack.org/#/c/293168/ ) doesn't seem to indicate why.

It was an attempt to keep the change more self-contained, and since 
UPDATE_HORIZON_CONFIG existed, it seemed reasonable to use it. It meant that 
all the configuration regarding the visibility of the panel was in one place, 
and since it's expected that deployers edit enabled files, I guess your concern 
stated above didn't come into it.

I'm ambivalent about the change you propose, would be OK going either way :-)


 Richard


__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [horizon] Angular panel enable/disable not overridable in local_settings

2016-08-02 Thread Richard Jones
On 3 August 2016 at 00:32, Rob Cresswell 
wrote:

> Hi all,
>
> So we seem to be adopting a pattern of using UPDATE_HORIZON_CONFIG in the
> enabled files to add a legacy/angular toggle to the settings. I don't like
> this, because in settings.py the enabled files are processed *after*
> local_settings.py imports, meaning the angular panel will always be
> enabled, and would require a local/enabled file change to disable it.
>
> My suggestion would be:
>
> - Remove current UPDATE_HORIZON_CONFIG change in the swift panel and
> images panel patch
> - Add equivalents ('angular') to the settings.py HORIZON_CONFIG dict, and
> then the 'legacy' version to the test settings.
>
> I think that should run UTs as expected, and allow the legacy/angular
> panel to be toggled via local_settings.
>
> Was there a reason we chose to use UPDATE_HORIZON_CONFIG, rather than just
> updating the dict in settings.py? I couldn't recall a reason, and the
> original patch ( https://review.openstack.org/#/c/293168/ ) doesn't seem
> to indicate why.
>

It was an attempt to keep the change more self-contained, and since
UPDATE_HORIZON_CONFIG existed, it seemed reasonable to use it. It meant
that all the configuration regarding the visibility of the panel was in one
place, and since it's expected that deployers edit enabled files, I guess
your concern stated above didn't come into it.

I'm ambivalent about the change you propose, would be OK going either way
:-)


 Richard
__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev