Change in vdsm[master]: Introduce commands package to separate execCmd and AsyncProc...

nsoffer at redhat.com nsoffer at redhat.com
Fri Jan 8 18:12:27 UTC 2016


Nir Soffer has posted comments on this change.

Change subject: Introduce commands package to separate execCmd and AsyncProc from utils
......................................................................


Patch Set 6:

(1 comment)

https://gerrit.ovirt.org/#/c/51421/6/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 44: from vdsm import qemuimg
Line 45: from vdsm import response
Line 46: from vdsm import supervdsm
Line 47: from vdsm import utils
Line 48: from vdsm.host import getHostUUID
> why? I like it that way
Because it breaks monkeypatching, and make it harder to understand dependencies.

We should import only modules, this should be the rule. Only stuff like contextlib.contextmanager should be imported from modules.
Line 49: from vdsm.compat import pickle
Line 50: from vdsm.config import config
Line 51: from vdsm.define import ERROR, NORMAL, doneCode, errCode
Line 52: from vdsm.netinfo import DUMMY_BRIDGE


-- 
To view, visit https://gerrit.ovirt.org/51421
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I295f5516154c464fe5b1fc103b7b9d1d76dffe17
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Edward Haas <edwardh at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Sahina Bose <sabose at redhat.com>
Gerrit-Reviewer: Shahar Havivi <shavivi at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list