Change in vdsm[master]: ifcfg: Log unhandled exception for new Thread

[Maor Lipchuk]

Maor Lipchuk has uploaded a new change for review.

Change subject: ifcfg: Log unhandled exception for new Thread
......................................................................

ifcfg: Log unhandled exception for new Thread

Ading a traceback log for unhandled exceptions,
when openning a new thread, so it will not die silently.
Since the log in ifcfg instance is inaccessible	from the decorator,
we use the default root logger.

Change-Id: I2ce44b4586e85438898fcdcd2d62d80813caa5ba
Signed-off-by: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
---
M vdsm/netconf/ifcfg.py
1 file changed, 1 insertion(+), 0 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/85/23085/1

diff --git a/vdsm/netconf/ifcfg.py b/vdsm/netconf/ifcfg.py
index 52b95b1..8431e80 100644
--- a/vdsm/netconf/ifcfg.py
+++ b/vdsm/netconf/ifcfg.py
@@ -738,6 +738,7 @@
 
 def ifup(iface, async=False):
     "Bring up an interface"
+    @utils.traceback()
     def _ifup(netIf):
         rc, out, err = utils.execCmd([constants.EXT_IFUP, netIf], raw=False)
 


-- 
To view, visit http://gerrit.ovirt.org/23085
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I2ce44b4586e85438898fcdcd2d62d80813caa5ba
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: ifcfg: Log unhandled exception for new Thread
......................................................................


Patch Set 1:

(1 comment)

....................................................
File vdsm/netconf/ifcfg.py
Line 737: 
Line 738: 
Line 739: def ifup(iface, async=False):
Line 740:     "Bring up an interface"
Line 741:     @utils.traceback()
I am not sure about the grand benefits of this: there's nothing in this function
besides calling execCmd, which does its logging anyway. All I see is duplicated log
noise.
Line 742:     def _ifup(netIf):
Line 743:         rc, out, err = utils.execCmd([constants.EXT_IFUP, netIf], raw=False)
Line 744: 
Line 745:         if rc != 0:


-- 
To view, visit http://gerrit.ovirt.org/23085
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ce44b4586e85438898fcdcd2d62d80813caa5ba
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: ifcfg: Log unhandled exception for new Thread
......................................................................


Patch Set 2:

Build Failed 

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/7075/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7977/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7866/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_network_functional_tests/1223/ : SUCCESS

-- 
To view, visit http://gerrit.ovirt.org/23085
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ce44b4586e85438898fcdcd2d62d80813caa5ba
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Itamar Heim <iheim(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: ifcfg: Log unhandled exception for new Thread
......................................................................


Patch Set 1:

(2 comments)

Looks good except "»" that sneaked in to the commit message.

....................................................
Commit Message
Line 7: ifcfg: Log unhandled exception for new Thread
Line 8: 
Line 9: Ading a traceback log for unhandled exceptions,
Line 10: when openning a new thread, so it will not die silently.
Line 11: Since the log in ifcfg instance is inaccessible	from the decorator,
»??
Line 12: we use the default root logger.
Line 13: 
Line 14: Change-Id: I2ce44b4586e85438898fcdcd2d62d80813caa5ba


....................................................
File vdsm/netconf/ifcfg.py
Line 737: 
Line 738: 
Line 739: def ifup(iface, async=False):
Line 740:     "Bring up an interface"
Line 741:     @utils.traceback()
This will help when you modify the code and break something. For example:

    rc, out, err = utils.exeCmd([constants.EXT_IFUP, netIf], raw=False)

Without this log, you will get silent failure. With this patch you could easily fix the
typo and have time to send Maor a thank you letter :-)

The log noise from execCmd is another issue that we should fix separately. It should be
the responsibility of the caller to log errors, not execCmd.
Line 742:     def _ifup(netIf):
Line 743:         rc, out, err = utils.execCmd([constants.EXT_IFUP, netIf], raw=False)
Line 744: 
Line 745:         if rc != 0:


-- 
To view, visit http://gerrit.ovirt.org/23085
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ce44b4586e85438898fcdcd2d62d80813caa5ba
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

Itamar Heim has posted comments on this change.

Change subject: ifcfg: Log unhandled exception for new Thread
......................................................................


Patch Set 1:

ping?

-- 
To view, visit http://gerrit.ovirt.org/23085
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ce44b4586e85438898fcdcd2d62d80813caa5ba
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Itamar Heim <iheim(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Antoni Segura Puimedon has posted comments on this change.

Change subject: ifcfg: Log unhandled exception for new Thread
......................................................................


Patch Set 1: Code-Review+1

-- 
To view, visit http://gerrit.ovirt.org/23085
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ce44b4586e85438898fcdcd2d62d80813caa5ba
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Itamar Heim has abandoned this change.

Change subject: ifcfg: Log unhandled exception for new Thread
......................................................................


Abandoned

no activity. restore if still relevant

-- 
To view, visit http://gerrit.ovirt.org/23085
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: abandon
Gerrit-Change-Id: I2ce44b4586e85438898fcdcd2d62d80813caa5ba
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: ifcfg: Log unhandled exception for new Thread
......................................................................


Patch Set 1:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/6652/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/5759/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_network_functional_tests/1132/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/6565/ : SUCCESS

-- 
To view, visit http://gerrit.ovirt.org/23085
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ce44b4586e85438898fcdcd2d62d80813caa5ba
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Itamar Heim has posted comments on this change.

Change subject: ifcfg: Log unhandled exception for new Thread
......................................................................


Patch Set 2:

ping

-- 
To view, visit http://gerrit.ovirt.org/23085
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ce44b4586e85438898fcdcd2d62d80813caa5ba
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Itamar Heim <iheim(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No