fix: tools: faulty mtrace lines do not fail script #1343
Conversation
ekurdybx
requested review from
a team,
golowanow,
lgirdwood and
marc-hb
as code owners
ekurdybx
force-pushed
the
fix-mtrace-ignore-faulty-lines
branch
from
85f5396 to
8f5f5a3
Compare
ekurdybx
changed the title
ekurdybx
force-pushed
the
fix-mtrace-ignore-faulty-lines
branch
2 times, most recently
from
e24b25e to
0fcba1f
Compare
| ctx, func = rest[0:2] | ||
| msg = ': '.join(rest[2:]).strip() | ||
| yield TraceItem(timestamp, trace_lvl, ctx, func, msg) | ||
| except Exception as e: # pylint: disable=W0718 |
There was a problem hiding this comment.
Ironic how W0718 warns about precisely the type of "green failure" issues that this PR is trying to fix... Why not simply catch the specific parsing exception?
There was a problem hiding this comment.
This script is a additional tool for getting stats from mtrace file, not a main test. That's why we decided to "ignore" lines that couldn't be parsed for whatever reason - hence the broad exception.
There was a problem hiding this comment.
You are missing the point. There is a limited number of parsing exception types this small piece of code can raise and that can be expected here. It would be easy to list them here. Catching any exception instead, including unexpected exceptions unrelated to parsing means hiding bugs in this code. This is what W0718 means.
Amusingly, this is a similar sort of issue that this PR is fixing.
tools/sof_perf_analyzer.py
Outdated
| hours=int(h), | ||
| minutes=int(m), | ||
| seconds=float(s) | ||
| ).total_seconds() |
There was a problem hiding this comment.
I think the timestamp parsing has become long enough to deserve its own, separate function. The nesting of try blocks and the significant indentation make the code less readable.
In case of a faulty line in mtrace, the sof_perf_analyzer.py script will log a warning and move on, instead of failing. Signed-off-by: Emilia Kurdybelska <emiliax.kurdybelska@intel.com>
ekurdybx
force-pushed
the
fix-mtrace-ignore-faulty-lines
branch
from
0fcba1f to
e06f756
Compare
4 participants
In case of a faulty mtrace line the sof_perf_analyzer.py script will log warning and move on, instead of failing the script (and the test).