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

Fixes #13632: Allow to report on same reporting parameters #840

Conversation

VinceMacBuche
Copy link
Member

@VinceMacBuche VinceMacBuche force-pushed the bug_13632/allow_to_report_on_same_reporting_parameters branch from 6c270ea to 8560cd9 Compare October 10, 2018 10:32
@VinceMacBuche
Copy link
Member Author

Commit modified

@VinceMacBuche VinceMacBuche force-pushed the bug_13632/allow_to_report_on_same_reporting_parameters branch from 8560cd9 to 975b36c Compare October 10, 2018 10:33
@VinceMacBuche
Copy link
Member Author

Commit modified

@@ -203,7 +203,7 @@ bundle agent _log_rudder_v2(expected_reports_source, message, class_prefix, args

methods:
report_data_found::
"any" usebundle => _rudder_common_reports_generic("${report_data[1]}", "${class_prefix}", "${report_data[3]}", "${promisers}", "${args_line1}", "${message_line1}"),
"any" usebundle => _rudder_common_reports_generic("${report_data[1]}", "${class_prefix}", "${report_data[3]}", "${promisers}", "${args_line1}", "${message_line1}",""),
Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe there should be a value here, i dunno ... but since logger_v2 is almost never called ...

Copy link
Member

@ncharles ncharles left a comment

Choose a reason for hiding this comment

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

Awesome ! This is some hell of cfengine-fu
Can you also update the others GMs ?

ifvarclass => "${class_prefix}_noop.!${class_prefix}_kept.!${class_prefix}_repaired.!${class_prefix}_error";

"success"
usebundle => _rudder_common_report("${technique_name}", "result_success", "${identifier}", "${component_name}", "${component_key}", "${message_prefix} was correct"),
"success-${join}"
Copy link
Member

Choose a reason for hiding this comment

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

you don't need to change this promiser

@@ -277,7 +277,8 @@ bundle agent _rudder_common_report(technique_name, status, identifier, component

reports:
!changes_only|send_reports::
"@@${technique_name}@@${status}@@${identifier}@@${component_name}@@${component_key}@@${g.execRun}##${g.uuid}@#${message}";
"@@${technique_name}@@${status}@@${identifier}@@${component_name}@@${component_key}@@${g.execRun}##${g.uuid}@#${message}"
comment => "${join}";
Copy link
Member

Choose a reason for hiding this comment

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

Can you improve the text by saying: "Method call with parameters ${join}", or something equivalent ?

@VinceMacBuche VinceMacBuche force-pushed the bug_13632/allow_to_report_on_same_reporting_parameters branch from 975b36c to 6586811 Compare October 10, 2018 13:37
@VinceMacBuche
Copy link
Member Author

Commit modified

Copy link
Member

@ncharles ncharles left a comment

Choose a reason for hiding this comment

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

That's great ! however, i think thal all file_ should be anonymised, and also you should do the "joined" for each of them

@@ -36,5 +36,5 @@ bundle agent file_ensure_lines_present(file, lines)
"enforce lines content" usebundle => file_enforce_content("${file}", "${lines}", "false");
"new result classes" usebundle => _classes_copy("${class_prefix}_enforce_lines_content", "${class_prefix}");

"report" usebundle => _log("Insert content ${lines} into ${file}", "", "${class_prefix}", @{args});
"report" usebundle => _log("Insert content into ${file}", "", "${class_prefix}", @{args});
Copy link
Member

Choose a reason for hiding this comment

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

you should also "joined" string => join(",",args); here

@VinceMacBuche
Copy link
Member Author

Commit modified

@VinceMacBuche VinceMacBuche force-pushed the bug_13632/allow_to_report_on_same_reporting_parameters branch from 6586811 to 27c57cd Compare October 11, 2018 11:02
@Normation-Quality-Assistant
Copy link
Contributor

This PR is not mergeable to upper versions.
Since it is "Ready for merge" you must merge it by yourself using the following command:
rudder-dev merge https://github.com/Normation/ncf/pull/840
-- Your faithful QA
Kant merge: "To be is to do."
(https://ci.normation.com/jenkins/job/merge-accepted-pr/2859/console)

@VinceMacBuche VinceMacBuche merged commit 63679de into Normation:branches/rudder/4.1 Oct 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants