[node-patches] Change in ovirt-node[master]: Add return value when persist command does not return an err...

[fabiand at fedoraproject.org]

Fabian Deutsch has posted comments on this change.

Change subject: Add return value when persist command does not return an error code on failure
......................................................................


Patch Set 2:

(2 comments)

Hey,

basically the code looks fine. But please use the built-in SystemExit exception.

Also ntoe that we are slowly transitioning away rom the ovirtnode.* namespace to use the new ovirt.node.* namespace, so we will probably move that function to the new namespace at some point.

http://gerrit.ovirt.org/#/c/22117/2/src/ovirtnode/exceptions.py
File src/ovirtnode/exceptions.py:

Line 19: """
Line 20: oVirtnode specific exceptions
Line 21: """
Line 22: 
Line 23: class ExceptionWithValue(Exception):
You might not need this exception as you can use the built-in SystemExit exception (and the .code member for the value)
Line 24:     def __init__(self, value):
Line 25:         self.value = value


http://gerrit.ovirt.org/#/c/22117/2/src/ovirtnode/ovirtfunctions.py
File src/ovirtnode/ovirtfunctions.py:

Line 37: import logging
Line 38: import grp
Line 39: import pwd
Line 40: from ovirt.node.config import defaults
Line 41: from ovirtnode.exceptions import *
Please avoid widlcard imports, as this breaks static checks.

Only import the class/exception you need

And as noted previoulsy - you can also just use the SystemExit exception.
Line 42: from ovirt.node.utils import process
Line 43: import ovirt.node.utils.system as osystem
Line 44: from ovirt.node.utils.console import TransactionProgress, Transaction
Line 45: from ovirtnode.network import *


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I014c41c9919cc9f8c260d85e0ef1a23ad17021a2
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-node
Gerrit-Branch: master
Gerrit-Owner: zhihui li <liyuran9522 at gmail.com>
Gerrit-Reviewer: Changming Bai <baichm at linux.vnet.ibm.com>
Gerrit-Reviewer: Fabian Deutsch <fabiand at fedoraproject.org>
Gerrit-Reviewer: Itamar Heim <iheim at redhat.com>
Gerrit-Reviewer: Wenyi Gao <wenyi at linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes