Skip to content
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

[cos]Fixed issue by adding valid plugin class and change CosLogs as a hierarchy of Independent logs #3287

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion sos/policies/distros/cos.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def __init__(self, sysroot=None, init=None, probe_runtime=True,
super(CosPolicy, self).__init__(sysroot=sysroot, init=init,
probe_runtime=probe_runtime,
remote_exec=remote_exec)
self.valid_subclasses += [CosPolicy]
self.valid_subclasses += [CosPlugin]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

curious, the valid_subclasses already has CosPlugin already at line 36

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line 36 needs to be removed, as it gets overwritten by the super() in __init__() from the base Policy class.


@classmethod
def check(cls, remote=''):
Expand Down
2 changes: 1 addition & 1 deletion sos/report/plugins/logs.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ class IndependentLogs(LogsBase, IndependentPlugin):
profiles = ('system', 'hardware', 'storage')


class CosLogs(LogsBase, CosPlugin):
class CosLogs(IndependentLogs, CosPlugin):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean that you don't want to be collecting all the logs defined in LogsBase as above?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will make it so that CosLogs has the IndependentPlugin tagging class assigned to it, which will enable it for all distros, which is precisely why we made the change to split out LogsBase.

The change to valid_subclasses is correct (and can be made "more" correct by removing line 36), but this change here is not going to get us where we need to be.

option_list = [
PluginOpt(name="log_days", default=3,
desc="the number of days logs to collect")
Expand Down