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 #9540: Create the expected reports file for ncf in system techniques instead of first log #1072
Conversation
# because the fist call to the logger may be in audit mode. | ||
"create ncf expected reports" usebundle => _create_current_expected_reports_file; | ||
# Aggregate the results of the two actions | ||
"combine result classes" usebundle => _classes_combine_two("ncf_init", "logger_rudder_final_resfile", "rudder_ncf_initialization") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if we should create another component or aggregate the data.
These components are not really understandable, but it is questionable to lose reporting granularity on agent side.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ha, this is a good question - i think both are ncf initialization, so we can surely combine them
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The global idea is correct, however i think you ought to change log_rudde.cf
"report about ncf init" usebundle => rudder_common_reports_generic("Common", "ncf_init", "&TRACKINGKEY&", "ncf Initialization", "None", "The ncf initialization"); | ||
"initialize ncf" usebundle => initialization; | ||
# Force generating expected reports file for Rudder logger | ||
# because the fist call to the logger may be in audit mode. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: "the first"
# because the fist call to the logger may be in audit mode. | ||
"create ncf expected reports" usebundle => _create_current_expected_reports_file; | ||
# Aggregate the results of the two actions | ||
"combine result classes" usebundle => _classes_combine_two("ncf_init", "logger_rudder_final_resfile", "rudder_ncf_initialization") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ha, this is a good question - i think both are ncf initialization, so we can surely combine them
"create ncf expected reports" usebundle => _create_current_expected_reports_file; | ||
# Aggregate the results of the two actions | ||
"combine result classes" usebundle => _classes_combine_two("ncf_init", "logger_rudder_final_resfile", "rudder_ncf_initialization") | ||
"report about ncf init" usebundle => rudder_common_reports_generic("Common", "rudder_ncf_initialization", "&TRACKINGKEY&", "ncf Initialization", "None", "The ncf initialization"); | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
aren't you missing a change in log_rudder.cf to not call _create_current_expected_reports_file ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is only called when needed and this PR is against 4.0, and we need to keep compatibility of ncf master with 3.2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is correct, thank you
…iques instead of first log
Commit modified |
5a4830e
to
f0fd18e
Compare
OK, merging this PR |
https://www.rudder-project.org/redmine/issues/9540