Skip to content
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

Feature/icingaweb module graphite #199

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

mkayontour
Copy link
Member

This adds the feature to install and configure Icinga Web 2 Module Graphite.

@mkayontour mkayontour linked an issue Oct 20, 2023 that may be closed by this pull request
@cla-bot cla-bot bot added the cla/signed label Oct 20, 2023
@gianmarco-mameli
Copy link
Contributor

Hi @mkayontour, there's already a release date for this feature? Thanks

@mkayontour
Copy link
Member Author

Sorry, I don't have a fixed release date - as I'm working on the collection only at spare times. I try to prepare a release before the summit. I can't promise you anything right now.

@mkayontour
Copy link
Member Author

I did a bit of cleanup and checked if the code is still compatible.

@mkayontour mkayontour requested a review from Donien November 13, 2024 21:46
@mkayontour mkayontour self-assigned this Nov 13, 2024
Copy link
Collaborator

@Donien Donien left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

converge needs the fix, the doc suggestion is...well, a suggestion.
LGTM 👍🏻

@@ -3,6 +3,54 @@
- name: Converge
hosts: all
vars:
icingaweb2_modules:
graphite:
enabled: true
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

source missing

Suggested change
enabled: true
enabled: true
source: package

Comment on lines +37 to +45
| insecure | number | "1" |

#### Section ui

| Variable | Type | Example |
|-------------------------|--------|---------|
| default_time_range | number | "1" |
| defautl_time_range_unit | string | "hours" |
| disable_no_graphs_found | bool | "0"/"1" |
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

insecure and disable_no_graphs_found should both either be number (or int) or bool, for consistency since both are essentially toggles.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Icinga Web: Add module graphite
3 participants