Code review comment for lp:~lazypower/charms/trusty/kibana/add-dashboard-loader-action

Revision history for this message
Cory Johns (johnsca) wrote :

The load-dashboards action param and dashboard config option descriptions should mention what dashboards are supported out of the box (i.e., "beats").

If the dashboard config option is left at its default value, the elasticsearch <-> kiabana relation is added, and then any other config option is changed, the config-changed hook fails because the config value is empty and it tries to call action-get from outside of an action. See inline comment below.

If the dashboard config is set before the elasticsearch relation is joined (likely in a bundle), the rest-relation- hooks will not trigger the dashboard load, and it may not be triggered until a manual config change.

If the elasticsearch relation is required for the dashboard to be loaded, why isn't the action enforcing that? Also, shouldn't the dashboard be using the relation values (e.g., $ES_HOST) instead of localhost?

review: Needs Fixing

« Back to merge proposal