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
1=== modified file 'config.yaml'
2--- config.yaml 2016-04-29 23:23:21 +0000
3+++ config.yaml 2016-05-06 22:05:36 +0000
4@@ -33,6 +33,14 @@
5 default: 'production'
6 type: string
7 description: 'production or development'
8+ anonymous:
9+ default: False
10+ type: boolean
11+ description: Whether to allow anonymous users, defaults to False.
12+ anonymous_role:
13+ default: 'Viewer'
14+ type: string
15+ description: The role given to anonymous users if enabled.
16 datasources:
17 default: ""
18 description: |
19
20=== modified file 'reactive/grafana.py'
21--- reactive/grafana.py 2016-05-04 15:28:47 +0000
22+++ reactive/grafana.py 2016-05-06 22:05:36 +0000
23@@ -69,14 +69,13 @@
24 hookenv.status_set('maintenance', 'Configuring grafana')
25 install_packages()
26 config = hookenv.config()
27- if data_changed('grafana.config', config):
28- settings = {'config': config}
29- render(source=GRAFANA_INI_TMPL,
30- target=GRAFANA_INI,
31- context=settings,
32- owner='root', group='grafana',
33- perms=0o640,
34- )
35+ settings = {'config': config}
36+ render(source=GRAFANA_INI_TMPL,
37+ target=GRAFANA_INI,
38+ context=settings,
39+ owner='root', group='grafana',
40+ perms=0o640,
41+ )
42 check_ports(config.get('port'))
43 set_state('grafana.start')
44 hookenv.status_set('active', 'Ready')
45
46=== modified file 'templates/grafana.ini.j2'
47--- templates/grafana.ini.j2 2016-02-24 19:08:53 +0000
48+++ templates/grafana.ini.j2 2016-05-06 22:05:36 +0000
49@@ -124,14 +124,20 @@
50 #################################### Anonymous Auth ##########################
51 [auth.anonymous]
52 # enable anonymous access
53+{% if config['anonymous'] -%}
54+enabled = true
55+
56+# specify role for unauthenticated users
57+org_role = {{ config['anonymous_role'] }}
58+{% else -%}
59 ;enabled = false
60
61+# specify role for unauthenticated users
62+;org_role = Viewer
63+{% endif %}
64 # specify organization name that should be used for unauthenticated users
65 ;org_name = Main Org.
66
67-# specify role for unauthenticated users
68-;org_role = Viewer
69-
70 #################################### Github Auth ##########################
71 [auth.github]
72 ;enabled = false

Subscribers

People subscribed via source and target branches

to all changes: