AnyAgent - improved logging to easier find bugs similar to bsc#1030425 #120
Conversation
agent-any/src/AnyAgentComplex.cc
Outdated
| if (tv.isNull()) | ||
| { | ||
| if (optional) | ||
| y2milestone("Optional Tuple not found at line: '%s'", line); |
There was a problem hiding this comment.
better to use y2debug, as it can be quite often case
agent-any/src/AnyAgentComplex.cc
Outdated
| if (optional) | ||
| y2milestone("Optional Tuple not found at line: '%s'", line); | ||
| else | ||
| y2error("Tuple not found at line: '%s'", line); |
There was a problem hiding this comment.
in general seeing output include newline. I think it will be better to strip it ( in all logging of line )
There was a problem hiding this comment.
Um, I'm not sure, the line might not contain a new line (the last line), the newline is processed by the agent definition so in some case you actually might know whether there is a new line or not.
Yes, it makes the logging a bit less readable but I'd rather prefer full info in the log.
|
@mvidner what do you think about this? I'm not sure whether the reported false positives are acceptable or not. See the updated tests. |
|
Um, let's close, it does not improve the situation much... |
This just increases the logging in the AnyAgent to easily find bugs similar to bsc#1030425.
Originally the parser silently aborted parsing without any trace in the log, debugging was then very tricky.
The Disadvantage
The increased logging on the other hand reports some false positives, e.g. the AnyAgent allows syntax like
:Or(:Tuple(...), :Tuple()). Which means it's OK when the first Tuple does not match the line, but it will log no match anyway as it cannot see the logic above.We could improve that logic, but IMHO that's waste of time as the AnyAgent is obsolete anyway and we should rather focus on using the new approach (Augeas).
So the question is: do we want this verbose logging at the price of the false positives? See the updated tests for some examples.