remove 'component' from generated modules and tests #196
Conversation
- also from UI and man page
|
@AloisMahdal Another issue emerged regarding this 'easy to fix' RFE. The UI is supposed to show a history of generated assessment reports. As @pirat89 found out, it seems that with this change the UI-related processing will not be able to parse the old reports correctly (old in terms of having 'component'). If you're able to test the UI whether it works correctly with both old and new reports this week, it would help. Otherwise, I'll drop this PR and revert the previously merged PR #195. Then it can sleep in a non-master branch waiting to be resurrected once we have more time to play with RFEs. Thanks. |
|
Nice catch. I'll be happy to test UI as well. |
|
By the way, What is the reason to remove 'component' from modules. Till now, each message is identified by component name and if it is wrong, like 'unknown' then module has to be fixed. The message should be somehow identified and we have to find out a way, how to identify it. I would prefer to focus to important things like #193, #184, #165, #131, #146. |
|
@phracek Just my mini input, because I don't remember it completely. |
|
This affects only INFO|ERROR|WARNING messages, not RISK messages. When I look into /var/log/preupgrade/preupg.log, the INFO|ERROR|WARNING messages are not there. They are just in the result.xml/html - and there the log messages are part of the module/rule context, so there's actually no need to add any component or module name info to the log message. |
|
@pirat89 If there are some unknown component names, then modules should be update properly. This is only information you do not specify 'COMPONENT' in your module. |
|
@phracek The reasoning behind this is in rhbz#1372871. In short, the component part is not necessary (it's implied by module and/or message text), makes log parsing harder and is never used by any module anyway. Also, while functional impact is not so significant, one of reasons for this is also to aid the cleanup goal by getting rid of unnecessary code: the idea is that by every (smaller, more focused and testable) step like this, we can shave off a little bit of complexity that makes refactoring so risky and painful. Regarding processing old vs. new reports in UI: We could support both formats. However I highly doubt that we have been keeping compatibility between reports, nobody ever tested this AFAIK. Also, given that the rule is to always use latest, freshest possible assessments (with official upgrade path this is even a hard requirement), the scenario where user needs to view an old report next to new one in UI does not seem very likely to me. |
|
|
||
| The function creates logs in the format: | ||
|
|
||
| <SEVERITIES> <component> <TIMESTAMP> <MESSAGE> | ||
| <SEVERITIES> <TIMESTAMP> <MESSAGE> |
There was a problem hiding this comment.
This is actually not true, there's no timestamp anymore. The result is that the log messages are not shown in UI in the current state and will not be after this update.
|
@phracek Which component should I set for module that serves 1000 rpms? Or set of all libraries? Or when module solves troubles for just two component... etc. There is not any specific component for such modules. And we have many modules like that. Should I set "distribution"? In that case the component doesn't have any usable information and "distribution" has same benefit as "unkown". Agree with Lojza in this. |
|
We tested this change with @AloisMahdal and it has no impact on UI. |
And also from UI and man page.
This PR continues upon the already merged PR #195.
Resolves: rhbz#1372871