Merge lp:~timkuhlman/charms/trusty/grafana/layer-grafana-http into lp:~canonical-is-sa/charms/trusty/grafana/layer-grafana

Proposed by Tim Kuhlman on 2016-05-06
Status: Merged
Merged at revision: 38
Proposed branch: lp:~timkuhlman/charms/trusty/grafana/layer-grafana-http
Merge into: lp:~canonical-is-sa/charms/trusty/grafana/layer-grafana
Diff against target: 72 lines (+24/-11)
3 files modified
config.yaml (+8/-0)
reactive/grafana.py (+7/-8)
templates/grafana.ini.j2 (+9/-3)
To merge this branch: bzr merge lp:~timkuhlman/charms/trusty/grafana/layer-grafana-http
Reviewer Review Type Date Requested Status
Jacek Nykis (community) 2016-05-06 Approve on 2016-05-09
Review via email: mp+294053@code.launchpad.net

Description of the change

Add optional anonymous access

To post a comment you must log in.
38. By Tim Kuhlman on 2016-05-06

Fixed template variables and double checking of config changed

Jacek Nykis (jacekn) wrote :

Do you know why data_changed does not work? If not I think we should investigate why because it should, we use this approach in other places too and it seems to work just fine there.

review: Needs Information
Tim Kuhlman (timkuhlman) wrote :

data_changed does work but reports on the data changed since the last time it was run not since the last hook. So what was happening was that when the config_changed hook would fire the function check_config would get called since grafana was already started. It does a data_changed call found out the data changed and would call setup_grafana, setup_grafana would run and do another data_changed call which found nothing had changed since the last run of the command in the check_config function and so would skip over the actual code update. By removing the second data_changed call I fixed that problem.

It is possible that on initial install or if grafana is not running setup_grafana will be called and then on the next config_changed hook check_config will call setup_grafana a second time. I didn't fully trace the logic through of that initial setup but thought a double config in that case is a lot better than failing to update after config-changed.

Jacek Nykis (jacekn) wrote :

Makes sense! LGTM

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'config.yaml'
--- config.yaml 2016-04-29 23:23:21 +0000
+++ config.yaml 2016-05-06 22:05:36 +0000
@@ -33,6 +33,14 @@
33 default: 'production'33 default: 'production'
34 type: string34 type: string
35 description: 'production or development'35 description: 'production or development'
36 anonymous:
37 default: False
38 type: boolean
39 description: Whether to allow anonymous users, defaults to False.
40 anonymous_role:
41 default: 'Viewer'
42 type: string
43 description: The role given to anonymous users if enabled.
36 datasources:44 datasources:
37 default: ""45 default: ""
38 description: |46 description: |
3947
=== modified file 'reactive/grafana.py'
--- reactive/grafana.py 2016-05-04 15:28:47 +0000
+++ reactive/grafana.py 2016-05-06 22:05:36 +0000
@@ -69,14 +69,13 @@
69 hookenv.status_set('maintenance', 'Configuring grafana')69 hookenv.status_set('maintenance', 'Configuring grafana')
70 install_packages()70 install_packages()
71 config = hookenv.config()71 config = hookenv.config()
72 if data_changed('grafana.config', config):72 settings = {'config': config}
73 settings = {'config': config}73 render(source=GRAFANA_INI_TMPL,
74 render(source=GRAFANA_INI_TMPL,74 target=GRAFANA_INI,
75 target=GRAFANA_INI,75 context=settings,
76 context=settings,76 owner='root', group='grafana',
77 owner='root', group='grafana',77 perms=0o640,
78 perms=0o640,78 )
79 )
80 check_ports(config.get('port'))79 check_ports(config.get('port'))
81 set_state('grafana.start')80 set_state('grafana.start')
82 hookenv.status_set('active', 'Ready')81 hookenv.status_set('active', 'Ready')
8382
=== modified file 'templates/grafana.ini.j2'
--- templates/grafana.ini.j2 2016-02-24 19:08:53 +0000
+++ templates/grafana.ini.j2 2016-05-06 22:05:36 +0000
@@ -124,14 +124,20 @@
124#################################### Anonymous Auth ##########################124#################################### Anonymous Auth ##########################
125[auth.anonymous]125[auth.anonymous]
126# enable anonymous access126# enable anonymous access
127{% if config['anonymous'] -%}
128enabled = true
129
130# specify role for unauthenticated users
131org_role = {{ config['anonymous_role'] }}
132{% else -%}
127;enabled = false133;enabled = false
128134
135# specify role for unauthenticated users
136;org_role = Viewer
137{% endif %}
129# specify organization name that should be used for unauthenticated users138# specify organization name that should be used for unauthenticated users
130;org_name = Main Org.139;org_name = Main Org.
131140
132# specify role for unauthenticated users
133;org_role = Viewer
134
135#################################### Github Auth ##########################141#################################### Github Auth ##########################
136[auth.github]142[auth.github]
137;enabled = false143;enabled = false

Subscribers

People subscribed via source and target branches

to all changes: