-
Notifications
You must be signed in to change notification settings - Fork 73
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
grafana.ini yaml syntax #232
base: main
Are you sure you want to change the base?
Conversation
|
49d5f29
to
31e23c1
Compare
cc @gardar |
I reviewed all vars to discover which are not used in role and can be descoped/deferred to grafana docs. Vars used in role:
Vars not used in role - removed from README.md and defaults/main.yaml in 2nd commit
Removed hardcoded sections in grafana.ini.j2 (could submit these as a separate PR, everything else is intertwined)
|
31e23c1
to
c4354a1
Compare
This is superb! Thanks @intermittentnrg LGTM, ill wait for sometime if @gardar want to review it and then merge |
c4354a1
to
3851f43
Compare
Finally got around to testing this. The dict grafana_ini gets overwritten and not inherited from defaults/main.yaml. ansible.builtin.set_fact:
grafana_ini: "{{ grafana_ini_default | ansible.builtin.combine(grafana_ini, recursive=true) }}" Also this line no longer works as it refers to itself. So just set this explictly. grafana_ini:
server:
root_url: "http://{{ grafana_ini.server.http_addr }}:{{ grafana_ini.server.http_port }}" I rebased and force pushed to fix conflicts. |
Untested draft for early feedback of #210.
I've done search&replace to use new convention for
grafana_ini:
var.I did not expect there to be 19 changed files, but there were many references for example to
grafana_url
which is nowgrafana_ini.server.root_url
and similar.I kept all entries in README and defaults/main.yaml, but I think most should be deleted. Repeating grafana documentation and default values doesn't make sense. The default grafana.ini is empty/everything commented out, we should let grafana handle defaults.