Fixes #9203: rudder_logger is too complex #464
Conversation
|
Do not merge, this is for feedback only |
|
One interesting value, for 77 reports in ncf, there is a gain of nearly half a second in the output |
|
ha, code is not working anymore :) |
|
ok, the gain is naught ... |
ncharles
force-pushed
the
bug_9203/rudder_logger_is_too_complex
branch
from
6a45810
to
c306e01
Compare
|
After trying to optimize the code further by not writing two files, i failed to obtain any gain compared to this implementation (which is more readable) |
|
This is ready for review |
| classes => classes_generic("logger_rudder_temp_resfile"); | ||
|
|
||
| # 3/ Write the final expected reports, expanded. First, empty it, then fill it | ||
| # (can't use edit_default empty, as we are iterating over a list) | ||
| !final_resfile_exists.logger_rudder_temp_resfile_repaired.!logger_rudder_final_resfile_repaired.keys_defined:: | ||
| "${expected_reports_file}" |
There was a problem hiding this comment.
We need to remove the file in case there is already one from a previous run with the same PID.
There was a problem hiding this comment.
by construction, the old file will be overwritten: if tmp file has been repaired, and expected_reports_file hasn't been repaired, then the expected_reports_file will be recreated
Ha, edit_defaults no_backup doesn't clean the file, i'm updating the PR
ncharles
force-pushed
the
bug_9203/rudder_logger_is_too_complex
branch
from
f84e44a
to
aeb8ceb
Compare
|
Commit modified |
|
This PR is not mergeable to upper versions. |
| edit_line => delete_lines_matching(".*"); | ||
| logger_rudder_temp_resfile_repaired.!logger_rudder_final_resfile_repaired.keys_defined:: | ||
| "${logger_rudder.expected_reports_file}" | ||
| delete => "tidy"; |
There was a problem hiding this comment.
this is tidy without quote
ncharles
force-pushed
the
bug_9203/rudder_logger_is_too_complex
branch
from
aeb8ceb
to
b72810f
Compare
|
Commit modified |
|
This PR is not mergeable to upper versions. |
|
OK, merging this PR |
|
OK, merging this PR |
2 similar comments
|
OK, merging this PR |
|
OK, merging this PR |
https://www.rudder-project.org/redmine/issues/9203