[node-patches] Change in ovirt-node[master]: Catch exceptions on EL6 for service calls

[fabiand at fedoraproject.org]

Fabian Deutsch has posted comments on this change.

Change subject: Catch exceptions on EL6 for service calls
......................................................................


Patch Set 3: Code-Review-1

(2 comments)

I think a cleaner approach is to not use a wrapper in this case. (Which also had a bug, it didin't return the func val).
This patch http://gerrit.ovirt.org/19621 is a suggestion of how to fix this issue. It's less invaisve and has doctests

....................................................
File src/ovirt/node/utils/process.py
Line 35:     "close_fds": True
Line 36: }
Line 37: 
Line 38: 
Line 39: class CalledProcessError(subprocess.CalledProcessError):
I wonder if we can avoid this wrapper class.

See below at pipe()
Line 40:     def __init__(self, returncode, cmd, output):
Line 41:         if 'output' in inspect.getargspec(
Line 42:                 subprocess.CalledProcessError.__init__):
Line 43:             super(CalledProcessError, self).__init__(returncode, cmd, output)


Line 120:     output = unicode(process.communicate(stdin)[0])
Line 121:     if process.returncode == 0:
Line 122:         return output
Line 123:     else:
Line 124:         raise CalledProcessError(process.returncode, cmd, output)
Can't we here just instantiate a CalledProcessError obj and then assign the required properties:

    err = CalledProcessError()
    err.output = ...
    err.cmd = ... # FIXME what's needed
    ...
    raise err

Is this enough?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia353ddcb912a304f5a9caca7ed40581ee79a92bf
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-node
Gerrit-Branch: master
Gerrit-Owner: Ryan Barry <rbarry at redhat.com>
Gerrit-Reviewer: Fabian Deutsch <fabiand at fedoraproject.org>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes