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.

Sign up for GitHub

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

Jump to bottom

Fixes #12517: Add generic methods to assign monitoring templates and parameters to nodes #745

Merged
merged 1 commit into from
Apr 24, 2018
Merged

Fixes #12517: Add generic methods to assign monitoring templates and parameters to nodes #745

merged 1 commit into from
Apr 24, 2018

Conversation

victorqrt
Copy link
Contributor

amousset
amousset reviewed Apr 23, 2018
View reviewed changes
tree/30_generic_methods/monitoring_parameter.cf Outdated
#####################################################################################

# @name Monitoring parameter
# @description Add a monitoring parameter to a node
Copy link
Member

Choose a reason for hiding this comment

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

We could use a @documentation section here, with a usage example. It should explain what a monitoring parameter is and it is used for. See other methods for syntax (the content itself is in matkdown)

Copy link
Member

Choose a reason for hiding this comment

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

add a (requires a monitoring plugin)

amousset
amousset reviewed Apr 23, 2018
View reviewed changes
tree/30_generic_methods/monitoring_template.cf Outdated
#####################################################################################

# @name Monitoring template
# @description Add a monitoring template to a node
Copy link
Member

Choose a reason for hiding this comment

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

same here

Copy link
Member

Choose a reason for hiding this comment

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

add a (requires a monitoring plugin)

amousset
amousset reviewed Apr 23, 2018
View reviewed changes
tree/30_generic_methods/monitoring_parameter.cf Outdated
@@ -0,0 +1,50 @@
#####################################################################################
# Copyright 2013 Normation SAS
Copy link
Member

Choose a reason for hiding this comment

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

2018

amousset
amousset reviewed Apr 23, 2018
View reviewed changes
tree/30_generic_methods/monitoring_template.cf Outdated
@@ -0,0 +1,48 @@
#####################################################################################
# Copyright 2013 Normation SAS
Copy link
Member

Choose a reason for hiding this comment

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

same here

@victorqrt
Copy link
Contributor Author

Commit modified

@victorqrt victorqrt force-pushed the ust_12517/add_generic_methods_to_assign_monitoring_templates_and_parameters_to_nodes branch from b62e3a9 to 1469664 Compare April 24, 2018 07:37
peckpeck
peckpeck reviewed Apr 24, 2018
View reviewed changes
tree/30_generic_methods/monitoring_parameter.cf Outdated
# are specific to each plugin and can be found in their respective documentation.
#
#
# @parameter key Name of the key
Copy link
Member

Choose a reason for hiding this comment

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

Name of the parameter

peckpeck
peckpeck reviewed Apr 24, 2018
View reviewed changes
tree/30_generic_methods/monitoring_parameter.cf
methods:

"disable_reporting_${old_class_prefix}" usebundle => disable_reporting;
"Write csv file" usebundle => file_ensure_lines_present("/var/rudder/tmp/rudder_monitoring.csv", "param,${key},${value}");
Copy link
Member

Choose a reason for hiding this comment

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

Is the case where value contains a comma handled properly ?

Copy link
Contributor Author

@victorqrt victorqrt Apr 24, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, the plugins will escape the commas past the second and the parameter will contain them as expected.

@victorqrt victorqrt force-pushed the ust_12517/add_generic_methods_to_assign_monitoring_templates_and_parameters_to_nodes branch from 1469664 to d8c34a9 Compare April 24, 2018 14:28
@victorqrt
Copy link
Contributor Author

Commit modified

@victorqrt victorqrt force-pushed the ust_12517/add_generic_methods_to_assign_monitoring_templates_and_parameters_to_nodes branch from d8c34a9 to d4d1193 Compare April 24, 2018 14:39
peckpeck
peckpeck requested changes Apr 24, 2018
View reviewed changes
tree/30_generic_methods/monitoring_parameter.cf Outdated
#
#
# @parameter key Name of the parameter
# @parameter value Value of the key
Copy link
Member

Choose a reason for hiding this comment

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

value of the parameter

@victorqrt
Copy link
Contributor Author

Commit modified

@victorqrt victorqrt force-pushed the ust_12517/add_generic_methods_to_assign_monitoring_templates_and_parameters_to_nodes branch from d4d1193 to 5560ffd Compare April 24, 2018 15:10
@Normation-Quality-Assistant
Copy link
Contributor

OK, merging this PR

@Normation-Quality-Assistant Normation-Quality-Assistant merged commit 5560ffd into Normation:branches/rudder/4.3 Apr 24, 2018
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@amousset amousset amousset left review comments

@peckpeck peckpeck peckpeck approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@victorqrt @Normation-Quality-Assistant @amousset @peckpeck