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

Add support for selecting service template #10

Merged
merged 3 commits into from
Jun 21, 2023
Merged

Conversation

pdorschner
Copy link

Implements configurable service template ref: vshn/signalilo#96

See the notes in the PR from vshn/signalilo#116

Copy link
Member

@simu simu left a comment

Choose a reason for hiding this comment

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

In the current state, this PR is not mergeable, as it moves a bunch of Signalilo-specific Service details into this library, which is intended to be a generic client for the Icinga2 API.

Additionally, the PR removes the ability to inspect a service's custom variables as returned from the Icinga2 API completely. This is a clear regression which needs to be addressed, see inline comments for options on how to do this.

icinga2/service.go Outdated Show resolved Hide resolved
icinga2/service.go Show resolved Hide resolved
icinga2/service.go Outdated Show resolved Hide resolved
icinga2/service.go Outdated Show resolved Hide resolved
@pdorschner
Copy link
Author

I've implemented new helper functions, which flattens the vars, so now it's possible to post all customvars in one level

@pdorschner
Copy link
Author

Hi,
is there any chance that this PR will be merged?

Best Regards

@simu
Copy link
Member

simu commented Feb 14, 2023

It's on my TODO list to review this PR and the corresponding Signalilo PR. I'll try to get it done this or next week.

Copy link
Member

@simu simu left a comment

Choose a reason for hiding this comment

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

Implementation LGTM now, except for the inline comment regarding the missing Vars contents when using GetService() or ListService().

icinga2/service.go Outdated Show resolved Hide resolved
icinga2/util.go Outdated Show resolved Hide resolved
@pdorschner
Copy link
Author

All issues have been fixed

@simu simu changed the title Implement configurable service template Add support for selecting service template Jun 21, 2023
@simu simu merged commit 0bebf56 into vshn:master Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants