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

Persist the interval in the conf file as a number #859

Merged
merged 2 commits into from Oct 10, 2018

Conversation

nbarrientos
Copy link
Contributor

When passing it as a string the plugin will basically ignore the setting and write the following to the log file:

  cf_util_get_cdtime:
     The Interval option requires exactly one numeric argument.

Without this patch it's not possible to declare different databases with different query interval.

When passing it as a string the plugin will basically ignore the
setting and write the following to the log file:

  cf_util_get_cdtime:
     The Interval option requires exactly one numeric argument.

Without this patch it's not possible to declare different databases with
different query interval.
@traylenator traylenator added bug Something isn't working needs-tests labels Oct 10, 2018
@traylenator
Copy link
Contributor

This parameter is not currently tested but a good time to add it.

@traylenator
Copy link
Contributor

Would be good to convert this particular plugin to puppet 4 types as well. The whole module expects puppet 4.

@nbarrientos
Copy link
Contributor Author

I've added an extra assert to an existing test to make sure that the value ends up in the configuration file properly formatted.

@alexjfisher
Copy link
Member

I'm happy with other changes eg. to add data types and replace create_resources to be done in a separate PR.

@traylenator traylenator merged commit 3fbab1e into voxpupuli:master Oct 10, 2018
@alexjfisher
Copy link
Member

@nbarrientos Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants