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 ping and steps configuration #173

Conversation

mvernimmen
Copy link

Database ping and steps can be changed

Pull Request (PR) description

This adds 2 parameters to configure database step and pings values.

This Pull Request (PR) fixes the following issues

n/a

Database ping and steps were not configurable because they were
hardcoded. Now it's a parameter.
@mvernimmen mvernimmen force-pushed the add_ping_and_step_configuration branch from 79982a1 to e6735d4 Compare August 1, 2024 13:20
Comment on lines +5 to +15
<% if @databasestep -%>
step = <%= @databasestep %>
<% else -%>
step = 300
<% end -%>

<% if @databasepings -%>
pings = <%= @databasepings %>
<% else -%>
pings = 20
<% end -%>
Copy link
Member

Choose a reason for hiding this comment

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

Aren't databasestep and databasepings always defined now?

@mvernimmen
Copy link
Author

mvernimmen commented Aug 1, 2024 via email

@kenyon
Copy link
Member

kenyon commented Aug 1, 2024

We should have tests instead of doing that. If you can add a test for the default content of /etc/smokeping/config.d/Database, and a test where non-default values are passed, that would be better.

@mvernimmen
Copy link
Author

I can interpret that in multiple ways. I other words, it's not entirely clear to me how you would like to have it. In my opinion the tests you mentioned are there in the template. shall I remove the defaults I added in manifests/init.pp ? Please help me understand better what you would like to see changed.

@kenyon
Copy link
Member

kenyon commented Aug 26, 2024

I can interpret that in multiple ways. I other words, it's not entirely clear to me how you would like to have it. In my opinion the tests you mentioned are there in the template. shall I remove the defaults I added in manifests/init.pp ? Please help me understand better what you would like to see changed.

By "tests" I meant rspec tests added to https://github.com/voxpupuli/puppet-smokeping/blob/34cf1bf28ab29c57c77e5bcde3fe96abf4d5ba53/spec/classes/init_spec.rb

@mvernimmen
Copy link
Author

I do not have the time/priority to learn how to write rspec tests to do this. Therefore I'm closing this PR.

@mvernimmen mvernimmen closed this Dec 30, 2024
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