Yeela Kaplan has uploaded a new change for review.
Change subject: sosreport: remove Base class ......................................................................
sosreport: remove Base class
Use of Base class inheritance breaks with sos>=3.1
Change-Id: I44816fdfe3197d655dda905c73a50e7c398b2e44 Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1167828 Signed-off-by: Yeela Kaplan ykaplan@redhat.com --- M vdsm/sos/vdsm.py.in 1 file changed, 8 insertions(+), 11 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/59/35759/1
diff --git a/vdsm/sos/vdsm.py.in b/vdsm/sos/vdsm.py.in index 535cb7a..02c6c17 100644 --- a/vdsm/sos/vdsm.py.in +++ b/vdsm/sos/vdsm.py.in @@ -20,12 +20,9 @@
try: from sos.plugins import Plugin, RedHatPlugin - - class Base(Plugin, RedHatPlugin): - pass except ImportError: import sos.plugintools - Base = sos.plugintools.PluginBase + Plugin = RedHatPlugin = sos.plugintools.PluginBase
import subprocess import os @@ -46,7 +43,7 @@ config = _importVdsmPylibModule('config').config
-class vdsm(Base): +class vdsm(Plugin, RedHatPlugin): """VDSM server related information """
@@ -54,12 +51,12 @@ False)]
# Make compatible com sos version >= 3 - if not hasattr(Base, 'addCopySpec'): - addCopySpec = Base.add_copy_spec - addCopySpecLimit = Base.add_copy_spec_limit - collectExtOutput = Base.add_cmd_output - getOption = Base.get_option - addForbiddenPath = Base.add_forbidden_path + if not hasattr(Plugin, 'addCopySpec'): + addCopySpec = Plugin.add_copy_spec + addCopySpecLimit = Plugin.add_copy_spec_limit + collectExtOutput = Plugin.add_cmd_output + getOption = Plugin.get_option + addForbiddenPath = Plugin.add_forbidden_path
def __addCopySpecLogLimit(self, path, logsize=None): """
Yeela Kaplan has posted comments on this change.
Change subject: sosreport: remove Base class ......................................................................
Patch Set 1: Verified+1
verified vdsm.log is created on:
sos-3.0-23.el7.noarch sos-2.2-47.el6.noarch sos-3.1-1.pkvm2_1.1.noarch
Sandro Bonazzola has posted comments on this change.
Change subject: sosreport: remove Base class ......................................................................
Patch Set 1:
(1 comment)
I would still prefer having 2 separate files avoiding this kind of workaround.
http://gerrit.ovirt.org/#/c/35759/1/vdsm/sos/vdsm.py.in File vdsm/sos/vdsm.py.in:
Line 42: # keep plugin's name for compatibility. Line 43: config = _importVdsmPylibModule('config').config Line 44: Line 45: Line 46: class vdsm(Plugin, RedHatPlugin): I would have expected an error like:
TypeError: Error when calling the metaclass bases duplicate base class sos.plugintools.PluginBase
looks weird it worked. Line 47: """VDSM server related information Line 48: """ Line 49: Line 50: optionList = [("logsize", 'max size (MiB) to collect per log file', '',
Dan Kenigsberg has posted comments on this change.
Change subject: sosreport: remove Base class ......................................................................
Patch Set 1: Code-Review+1
The big refactoring request should keep open. We need to find a hack-less solution that does not involve code duplication.
Imo, this patch can go in as it is, since the foctionality is needed urgently.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: sosreport: remove Base class ......................................................................
Patch Set 1:
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/13813/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/13024/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/13976/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_infra_functional_tests_gerrit/26/ : There was an infra issue, please contact infra@ovirt.org
Sandro Bonazzola has posted comments on this change.
Change subject: sosreport: remove Base class ......................................................................
Patch Set 1: Code-Review+1
Imo, this patch can go in as it is, since the foctionality is needed urgently.
If it works, ok for me.
Yeela Kaplan has posted comments on this change.
Change subject: sosreport: remove Base class ......................................................................
Patch Set 1: Verified-1
(1 comment)
http://gerrit.ovirt.org/#/c/35759/1/vdsm/sos/vdsm.py.in File vdsm/sos/vdsm.py.in:
Line 42: # keep plugin's name for compatibility. Line 43: config = _importVdsmPylibModule('config').config Line 44: Line 45: Line 46: class vdsm(Plugin, RedHatPlugin):
I would have expected an error like:
Thanks Sandro, I was suspicious I did not run into this kind of problem as well. It only happens when calling super(vdsm).
I'll work on the separation of the sos2 and sos3 classes solution. Line 47: """VDSM server related information Line 48: """ Line 49: Line 50: optionList = [("logsize", 'max size (MiB) to collect per log file', '',
Dan Kenigsberg has posted comments on this change.
Change subject: sosreport: remove Base class ......................................................................
Patch Set 1:
(1 comment)
http://gerrit.ovirt.org/#/c/35759/1/vdsm/sos/vdsm.py.in File vdsm/sos/vdsm.py.in:
Line 42: # keep plugin's name for compatibility. Line 43: config = _importVdsmPylibModule('config').config Line 44: Line 45: Line 46: class vdsm(Plugin, RedHatPlugin):
Thanks Sandro,
Apparently the suggested code flies because sos.plugintools.PluginBase is an old-style class.
If this code is functionally working, let's take it in, and work on the sos2/3 split right after thatn. Line 47: """VDSM server related information Line 48: """ Line 49: Line 50: optionList = [("logsize", 'max size (MiB) to collect per log file', '',
Yeela Kaplan has posted comments on this change.
Change subject: sosreport: remove Base class ......................................................................
Patch Set 1: Verified+1
returning the verified flag as sos.plugintools.PluginBase is an old-style class, which is the reason we did not get the TypeError.
This hack will work now as an urgent solution, will work on a new patch to separate the vdsm plugins for sos2/3.
Sandro Bonazzola has posted comments on this change.
Change subject: sosreport: remove Base class ......................................................................
Patch Set 1:
ok for me
Dan Kenigsberg has posted comments on this change.
Change subject: sosreport: remove Base class ......................................................................
Patch Set 1: Code-Review+2
Dan Kenigsberg has submitted this change and it was merged.
Change subject: sosreport: remove Base class ......................................................................
sosreport: remove Base class
Use of Base class inheritance breaks with sos>=3.1
Change-Id: I44816fdfe3197d655dda905c73a50e7c398b2e44 Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1167828 Signed-off-by: Yeela Kaplan ykaplan@redhat.com Reviewed-on: http://gerrit.ovirt.org/35759 Reviewed-by: Sandro Bonazzola sbonazzo@redhat.com Reviewed-by: Dan Kenigsberg danken@redhat.com --- M vdsm/sos/vdsm.py.in 1 file changed, 8 insertions(+), 11 deletions(-)
Approvals: Sandro Bonazzola: Looks good to me, but someone else must approve Yeela Kaplan: Verified Dan Kenigsberg: Looks good to me, approved
oVirt Jenkins CI Server has posted comments on this change.
Change subject: sosreport: remove Base class ......................................................................
Patch Set 2:
Build Failed
http://jenkins.ovirt.org/job/vdsm_master-libfapi_create-rpms-el6-x86_64_merg... : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_create-rpms-fc21-x86_64_merged/271/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master-libfapi_create-rpms-fc20-x86_64_mer... : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_create-rpms_merged_test_debug/488/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master-libfapi_create-rpms-el7-x86_64_merg... : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_create-rpms-fc20-x86_64_merged/286/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_merged/4280/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_create-rpms-el6-x86_64_merged/293/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_create-rpms-el7-x86_64_merged/295/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master-libfapi_create-rpms-fc21-x86_64_mer... : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_verify-error-codes_merged/6118/ : SUCCESS
vdsm-patches@lists.fedorahosted.org