Code review comment for ~tcuthbert/charm-grafana:master

Revision history for this message
Alvaro Uria (aluria) wrote :

In general, change looks good. However, functional tests fail because bionic-snap and focal-snap gate tests specify "snap_channel: 6/stable". I think that line should be removed and use the default (7/stable).

Several comments:
* Change in Grafana v7 was added here [1]
* data_source table schemas for v6 and v7 show that v7 has "uid" (see [2])
* Default APT package is v7
* Default snap_channel is 7/stable

This change will break install_method=snap and snap_channel=6/edge or snap_channel=stable (which is latest/stable and currently points to 6/stable revision).

The path forward will be to:
* merge and release (once the func tests that use install_method=snap are fixed) in cs:~llama-charmers/grafana
* For stable/20.08 release, we will make latest/stable match 7/stable (otherwise, this fix would need to track which version of grafana is in use, and if it is v6, then skip adding "uid" in generate_query function)

1. https://github.com/grafana/grafana/pull/23585
2. https://pastebin.ubuntu.com/p/jGb72gDzzZ/

review: Needs Fixing

« Back to merge proposal