Prometheus2 charm

Merge ~xavpaice/charm-prometheus2:add_default_rules into charm-prometheus2:master

  1. Git
  2. lp:~xavpaice/charm-prometheus2
  3. add_default_rules
  4. Merge into master
Proposed by Xav Paice
Status: Merged
Approved by: Paul Goins
Approved revision: 848846c61e680bbe6e6a35edc0537bdfa3e71073
Merged at revision: 848846c61e680bbe6e6a35edc0537bdfa3e71073
Proposed branch: ~xavpaice/charm-prometheus2:add_default_rules
Merge into: charm-prometheus2:master
Diff against target: 134 lines (+69/-1)
4 files modified
config.yaml (+21/-0)
reactive/prometheus.py (+28/-1)
templates/generic.rules.j2 (+14/-0)
unit_tests/test_reactive_prometheus.py (+6/-0)
Reviewer Review Type Date Requested Status
Paul Goins Approve
Alvaro Uria (community) Needs Fixing
Wouter van Bommel (community) Approve
Review via email: mp+382719@code.staging.launchpad.net
To post a comment you must log in.
Revision history for this message
Wouter van Bommel (woutervb) wrote :

Lgtm

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

I've added a comment inline re: wait_time hard-coding.

OTOH, when the prometheus2 application is upgraded, the generic rule won't be created because of line 458:
"""
if any(config.changed(opt) for opt in rules_change_opts) \
       or templates_changed([PATHS['refresh_rules_script_tmpl']]):
        configure_rules()
"""

I think the condition should also take into consideration PATHS["generic_rules_tmpl"].

Other than the above, it looks good.

review: Needs Fixing
Revision history for this message
Xav Paice (xavpaice) wrote :

Thanks for the detailed review - I've updated the change as discussed.

Revision history for this message
Paul Goins (vultaire) wrote :

LGTM. Approved.

review: Approve

There was an error fetching revisions from git servers. Please try again in a few minutes. If the problem persists, contact Launchpad support.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: